Merge pull request #4 from fiverr/fix_nodeclass_and_readme

Bug fixes and additionalRequirements support
fiverr · Apr 15, 2024 · 4ae9e24 · 4ae9e24
2 parents cf80be4 + 491d490
commit 4ae9e24
Show file tree

Hide file tree

Showing 13 changed files with 266 additions and 66 deletions.
diff --git a/README.md b/README.md
@@ -17,8 +17,8 @@ You can then run `helm search repo fiverr_public` to see the charts.
 
 ## License
 
-<!-- Keep full URL links to repo files because this README syncs from main to gh-pages.  -->
-[Apache 2.0 License](https://github.com/fiverr/public_charts/blob/main/LICENSE).
+<!-- Keep full URL links to repo files because this README syncs from master to gh-pages.  -->
+[Apache 2.0 License](https://github.com/fiverr/public_charts/blob/master/LICENSE).
 
 ## Helm charts build status
 

diff --git a/charts/karpenter_nodes/Chart.yaml b/charts/karpenter_nodes/Chart.yaml
@@ -1,6 +1,6 @@
 apiVersion: v2
 name: karpenter_nodes
-version: 0.0.2
+version: 0.1.0
 description: A Helm chart for generating NodeClasses and NodePools for Karpenter
 maintainers:
   - name: nadavbuc
diff --git a/charts/karpenter_nodes/README.md b/charts/karpenter_nodes/README.md
@@ -23,13 +23,13 @@ The `UserData` field supports templating and your own values. You can take a loo
 
 ### Testing Your Changes
 After making changes you will probably want to see the new output. Run `helm template` with the relevant example files: </br>
-`helm template . -f values.yaml`
+`helm template <some-name> . -f values.yaml`
 
 ### Unit Tests
 Make sure you have `helm-unittest` plugin installed. [helm-unittest](https://github.com/helm-unittest/helm-unittest)
 
 Unit tests are written in `tests` directory. To run the tests, use the following command: </br>
-`helm unittest --helm3 karpenter_nodes -f "tests/$value/*_test.yaml"`
+`helm unittest --helm3 karpenter_nodes -f "tests/*_test.yaml"`
 
 
 ## Configuration keys
@@ -46,7 +46,6 @@ Note - Most of the values can be overridden per nodegroup (If not specified, it
 | `securityGroupSelectorTerms`   | Selector for Security Groups [Documentation](https://karpenter.sh/docs/concepts/nodeclasses/#specsecuritygroupselectorterms) | `List(Map)` | x | ✓ |
 | `nodeGroupLabelName`           | The Name of the label for each nodegroup (default is `nodegroup`) | `String` | x | ✓ |
 | `nodeTags`                     | Tags to add to the instances `<tag_name>`: `<tag_value>` | `Map` | ✓ | ✓ |
-| `additionalNodeTags`           | Additional Tags to add to the instances `<tag_name>`: `<tag_value>` | `Map` | ✓ | ✓ |
 | `nodegroups.{}`                | each will be used to setup a provisioner and template based on the nodegrup name key | `List[Maps]` | x | ✓ |
 | `blockDeviceMappings`          | Block Device Mappings [Documentation](https://karpenter.sh/docs/concepts/nodeclasses/#specblockdevicemappings) | `List(Map)` | x | ✓ |
 | `detailedMonitoring`           | Detailed Monitoring [Documentation](https://karpenter.sh/docs/concepts/nodeclasses/#specdetailedmonitoring) | `Boolean` | x | ✓ |
@@ -72,11 +71,13 @@ Note - Most of the values can be overridden per nodegroup (If not specified, it
 | `consolidateAfter`             | Specify how long to wait before consolidating nodes [Documentation](https://karpenter.sh/docs/concepts/nodepools/) | `String` | ✓ | ✓ |
 | `excludeInstanceSize`          | Exclude specific instance sizes | `List` | ✓ | ✓ |
 | `headRoom`                     | Generate Ultra Low Priority Class for Headroom (see below) | `String` | ✓ | x |
+| `additionalRequirements`       | add NodePool requirements which are not covered by this chart | `List(map)` | ✓ | ✓ |
 
 ### NodeGroup Configuration
 |  Key Name                      | Description | Type | Optional? | Optional Per NodeGroup? |
 | ------------------------------ | ----------- | ---- | --------- | ----------------------- |
 | `nodegroups.{}.labels`         | Labels to add to nodes `<label_name>`: `<label_value>` | `Map` | ✓ | ✓ |
+| `nodegroups.{}.additionalNodeTags` | Additional Tags to add to the instances `<tag_name>`: `<tag_value>` | `Map` | ✓ | ✓ |
 | `nodegroups.{}.annotations`    | Annotations to add to nodes `<annotation_name>`: `<annotation_value>` | `Map` | ✓ | ✓ |
 | `nodegroups.{}.nodeClassRef`   | If you wish to use your own nodeClass, specify it [Documentation](https://karpenter.sh/docs/concepts/nodeclasses/) | `Map` | ✓ | ✓ |
 | `nodegroups.{}.taints`         | Taints to add to nodes `- <taint_key>`: `<taint_value>`: `<taint_effect>` | `List(Map)` | ✓ | ✓ |
@@ -96,7 +97,7 @@ The pods will be configured with ultra-low priority, and will be terminated and
 | `nodegroups.{}.headRoom`       | List of headroom configurations for the nodePool | `List(Map)` | ✓ | ✓ |
 | `nodegroups.{}.headRoom.size`  | `small`, `medium`, `large`, `xlarge` - see below | `String` | ✓ | ✓ |
 | `nodegroups.{}.headRoom.count` | Number of headroom pod replicas to schedule | `Integer` | ✓ | ✓ |
-| `nodegroups.{}.headRoom.antiAffinitySpec` | Required - set antiaffinity to match against all running workloads | `LabelSelectorSpec` | ✓ | ✓ |
+| `nodegroups.{}.headRoom.antiAffinitySpec` | Optional - set antiaffinity to match against running workloads | `LabelSelectorSpec` | ✓ | ✓ |
 | `nodegroups.{}.headRoom.nameSpaces` | Specify list of namespaces to match again (default `all`) | `List(String)` | ✓ | ✓ |
 
 ### Headroom Sizing

diff --git a/charts/karpenter_nodes/templates/headroom.yaml b/charts/karpenter_nodes/templates/headroom.yaml
@@ -38,7 +38,9 @@ spec:
                 operator: In
                 values:
                 - headroom-{{ $k }}-{{ $v.instances.architecture | default $.Values.instances.architecture }}-{{ $nhr.size }}
+            {{- if hasKey $nhr "antiAffinitySpec" }}
             {{- $nhr.antiAffinitySpec | toYaml | nindent 14 }}
+            {{- end }}
             topologyKey: kubernetes.io/hostname
             {{- if hasKey $nhr "nameSpaces" }}
             namespaces:

diff --git a/charts/karpenter_nodes/templates/nodeclass.yaml b/charts/karpenter_nodes/templates/nodeclass.yaml
@@ -1,6 +1,7 @@
 {{- range $k, $v := $.Values.nodeGroups }}
 {{- $_ := set $ "key" $k }}
 {{- $_ := set $ "value" $v }}
+{{- if not (hasKey $v "nodeClassRef") }}
 ---
 apiVersion: karpenter.k8s.aws/{{ $.Values.ApiVersion }}
 kind: EC2NodeClass
@@ -20,10 +21,8 @@ spec:
   {{- end }}
   amiFamily: {{ $v.amiFamily | default $.Values.amiFamily }}
   amiSelectorTerms:
-  {{- if hasKey $v "amiSelectorTerms" }}
-    {{- toYaml $v.amiSelectorTerms | nindent 4 }}
-  {{- else }}
-    {{- toYaml $.Values.amiSelectorTerms | nindent 4 }}
+  {{- if or (hasKey $v "amiSelectorTerms") (hasKey $.Values "amiSelectorTerms") }}
+    {{- toYaml ($v.amiSelectorTerms | default $.Values.amiSelectorTerms) | nindent 4 }}
   {{- end }}
   subnetSelectorTerms:
   {{- if hasKey $v "subnetSelectorTerms" }}
@@ -44,7 +43,7 @@ spec:
     {{- toYaml ($v.nodeTags | default $.Values.nodeTags) | nindent 4 }}
     {{- end }}
     {{- if hasKey $v "additionalNodeTags" }}
-    {{- toYaml $v.tags | nindent 4 }}
+    {{- toYaml $v.additionalNodeTags | nindent 4 }}
     {{- end }}
     managed_by: karpenter
   blockDeviceMappings:
@@ -74,3 +73,4 @@ spec:
   {{- end }}
   {{- end}}
 {{- end }}
+{{- end }}
diff --git a/charts/karpenter_nodes/templates/nodepool.yaml b/charts/karpenter_nodes/templates/nodepool.yaml
@@ -24,7 +24,7 @@ spec:
     spec:
       nodeClassRef:
         {{- if hasKey $v "nodeClassRef" }}
-          {{ $v.nodeClassRef | toYaml | nindent 8 }}
+          {{- $v.nodeClassRef | toYaml | nindent 8 }}
         {{- else }}
         name: {{ $k }}-{{ $v.instances.architecture | default $.Values.instances.architecture }}
         {{- end }}
@@ -102,20 +102,6 @@ spec:
           {{- end }}
           {{- end }}
         {{- end }}
-        {{- if or (hasKey $.Values "excludeCpuManufacturer") (hasKey $v "excludeCpuManufacturer") }}
-        - key: "karpenter.k8s.aws/instance-cpu-manufacturer"
-          operator: NotIn
-          values:
-          {{- if hasKey $v "excludeCpuManufacturer" }}
-          {{- range $v .excludeCpuManufacturer }}
-          - {{ . }}
-          {{- end }}
-          {{- else }}
-          {{- range $.Values.excludeCpuManufacturer }}
-          - {{ . }}
-          {{- end }}
-          {{- end }}
-        {{- end }}
         {{- if or (hasKey $.Values "excludeInstanceSize") (hasKey $v "excludeInstanceSize") }}
         - key: "karpenter.k8s.aws/instance-size"
           operator: NotIn
@@ -146,6 +132,9 @@ spec:
             - {{ . -}}
           {{- end }}
         {{- end }}
+        {{- if or (hasKey $.Values "additionalRequirements") (hasKey $v "additionalRequirements") }}
+        {{- toYaml ($v.additionalRequirements | default $.Values.additionalRequirements) | nindent 8 }}
+        {{- end }}
       kubelet:
         {{- if or (hasKey $v "kubeletClusterDNS") (hasKey $.Values "kubeletClusterDNS") }}
         clusterDNS:
@@ -209,7 +198,7 @@ spec:
     consolidateAfter: {{ $v.consolidateAfter | default $.Values.consolidateAfter }}
     {{- if $v.budgets }}
     budgets:
-    {{- $v.budgets | toYaml | nindent 6 }}
+    {{- toYaml $v.budgets | nindent 6 }}
     {{- end }}
   {{- if hasKey $v "limits" }}
   limits:

diff --git a/charts/karpenter_nodes/tests/headroom_nodes_default_test.yaml b/charts/karpenter_nodes/tests/headroom_nodes_default_test.yaml
@@ -39,12 +39,6 @@ tests:
       - equal:
           path: spec.template.spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].labelSelector.matchExpressions[0].values[0]
           value: headroom-nodes-default-amd64-small
-      - equal:
-          path: spec.template.spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].labelSelector.matchExpressions[1].key
-          value: testlabel1
-      - equal:
-          path: spec.template.spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].labelSelector.matchExpressions[1].operator
-          value: Exists
       - equal:
           path: spec.template.spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].namespaces
           value: {}

diff --git a/charts/karpenter_nodes/tests/nodeclass_test.yaml b/charts/karpenter_nodes/tests/nodeclass_test.yaml
@@ -1,10 +1,13 @@
 suite: test nodeclasses
 templates:
   - nodeclass.yaml
-
 values:
   - values.yaml
 tests:
+  - it: should only render 2 nodeclasses
+    asserts:
+    - hasDocuments:
+        count: 2
   - it: Verify nodes-default
     documentIndex: 0
     asserts:
@@ -80,6 +83,15 @@ tests:
       - equal:
           path: spec.amiFamily
           value: BottleRocket
+      - equal:
+          path: spec.amiSelectorTerms[0].tags.environment
+          value: test
+      - equal:
+          path: spec.amiSelectorTerms[1].name
+          value: my-ami
+      - equal:
+          path: spec.amiSelectorTerms[2].id
+          value: ami-123
       - equal:
           path: spec.subnetSelectorTerms[0].tags.Name
           value: eks-subnet-workers-1
@@ -95,6 +107,12 @@ tests:
       - equal:
           path: spec.tags.cluster
           value: eks-dev
+      - equal:
+          path: spec.tags.testtag1
+          value: tag1
+      - equal:
+          path: spec.tags.testtag2
+          value: tag2
       - equal:
           path: spec.blockDeviceMappings[0].deviceName
           value: /dev/xvda

diff --git a/charts/karpenter_nodes/tests/nodepool_nodes_default_test.yaml b/charts/karpenter_nodes/tests/nodepool_nodes_default_test.yaml
@@ -1,7 +1,6 @@
 suite: test nodepool - nodes-default
 templates:
   - nodepool.yaml
-
 values:
   - values.yaml
 
@@ -52,7 +51,9 @@ tests:
   - it: Verify nodes-default requirements
     documentIndex: 0
     asserts:
-    # instance-category
+      - equal:
+          path: spec.template.spec.requirements[0].key
+          value: "karpenter.k8s.aws/instance-category"
       - notContains:
           path: spec.template.spec.requirements[0].values
           content: t
@@ -62,47 +63,63 @@ tests:
       - equal:
           path: spec.template.spec.requirements[0].values[2]
           value: c
-    # instance-cpu
+      - equal:
+          path: spec.template.spec.requirements[1].key
+          value: "karpenter.k8s.aws/instance-cpu"
       - equal:
           path: spec.template.spec.requirements[1].values[0]
           value: "4"
       - equal:
           path: spec.template.spec.requirements[1].values[2]
           value: "16"
-    # instance-generation
+      - equal:
+          path: spec.template.spec.requirements[2].key
+          value: "karpenter.k8s.aws/instance-generation"
       - equal:
           path: spec.template.spec.requirements[2].operator
           value: "Gt"
       - equal:
           path: spec.template.spec.requirements[2].values[0]
           value: "6"
-    # instance-zone
+      - equal:
+          path: spec.template.spec.requirements[3].key
+          value: "topology.kubernetes.io/zone"
       - equal:
           path: spec.template.spec.requirements[3].values[0]
           value: "eu-west-1a"
       - equal:
           path: spec.template.spec.requirements[3].values[2]
           value: "eu-west-1c"
-    # instance-architecture
+      - equal:
+          path: spec.template.spec.requirements[4].key
+          value: "kubernetes.io/arch"
       - equal:
           path: spec.template.spec.requirements[4].values[0]
           value: "amd64"
-    # instance-capacity-type
+      - equal:
+          path: spec.template.spec.requirements[5].key
+          value: "karpenter.sh/capacity-type"
       - equal:
           path: spec.template.spec.requirements[5].values[0]
           value: "spot"
       - equal:
           path: spec.template.spec.requirements[5].values[1]
           value: "on-demand"
-    # instance-OS
+      - equal:
+          path: spec.template.spec.requirements[6].key
+          value: "kubernetes.io/os"
       - equal:
           path: spec.template.spec.requirements[6].values[0]
           value: "linux"
-    # instance-family Exclusions
+      - equal:
+          path: spec.template.spec.requirements[7].key
+          value: "karpenter.k8s.aws/instance-family"
       - equal:
           path: spec.template.spec.requirements[7].values[0]
           value: "c6a"
-    # instance-size Exclusiong
+      - equal:
+          path: spec.template.spec.requirements[8].key
+          value: "karpenter.k8s.aws/instance-size"
       - equal:
           path: spec.template.spec.requirements[8].operator
           value: NotIn
@@ -133,7 +150,7 @@ tests:
           path: spec.disruption.consolidateAfter
           value: 5m
       - isNull:
-          path: spec.budgets
+          path: spec.disruption.budgets
       - isNull:
           path: spec.limits
       - equal: