Repository: kubernetes/kops (go)

Issue:
karpenter-controller CPU request is way too high and not spread across nodes
In the addon, karpenter-controller reserves [half a CPU](https://github.com/kubernetes/kops/blob/3bafdc414288b7525bbdf67e93d6ccfa5bde091d/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template#L883). Looking at my own stats, it barely needs more than 2%.  

![image](https://user-images.githubusercontent.com/342823/214046958-46f958a2-e4f9-42f7-877e-f331c1a2cad4.png)

Additionally the controller deployment is using the wrong `topologySpreadConstraint` key (`webhook` instead of `controller`): https://github.com/kubernetes/kops/blob/3bafdc414288b7525bbdf67e93d6ccfa5bde091d/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template#L948-L960

Pull request description:
aws: Configure Karpenter resources in the cluster spec
Allow configuration of the CPU request, memory request, and memory limit for Karpenter pods in the cluster spec.

This PR also changes the default request for CPU from 500m to 25m and for memory from 1Gi to 200Mi (the memory limit is unchanged at 1Gi). Is this too harsh? Would it be better to err on the side of higher requests and risk wasting resources on smaller clusters or err on the side of lower requests and risk starving pods on larger clusters?

Closes #15046.

Patch:
diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml
index bfb30f0f42..784263873b 100644
--- a/k8s/crds/kops.k8s.io_clusters.yaml
+++ b/k8s/crds/kops.k8s.io_clusters.yaml
@@ -1505,6 +1505,12 @@ spec:
               karpenter:
                 description: Karpenter defines the Karpenter configuration.
                 properties:
+                  cpuRequest:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                    x-kubernetes-int-or-string: true
                   enabled:
                     type: boolean
                   image:
@@ -1513,6 +1519,18 @@ spec:
                     type: string
                   logLevel:
                     type: string
+                  memoryLimit:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                    x-kubernetes-int-or-string: true
+                  memoryRequest:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                    x-kubernetes-int-or-string: true
                 type: object
               keyStore:
                 description: KeyStore is the VFS path to where SSL keys and certificates
diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go
index d594e4380a..06a9cec8be 100644
--- a/pkg/apis/kops/cluster.go
+++ b/pkg/apis/kops/cluster.go
@@ -254,10 +254,13 @@ type ScalewaySpec struct {
 }
 
 type KarpenterConfig struct {
-	Enabled     bool   `json:"enabled,omitempty"`
-	LogEncoding string `json:"logFormat,omitempty"`
-	LogLevel    string `json:"logLevel,omitempty"`
-	Image       string `json:"image,omitempty"`
+	Enabled       bool               `json:"enabled,omitempty"`
+	LogEncoding   string             `json:"logFormat,omitempty"`
+	LogLevel      string             `json:"logLevel,omitempty"`
+	Image         string             `json:"image,omitempty"`
+	MemoryLimit   *resource.Quantity `json:"memoryLimit,omitempty"`
+	MemoryRequest *resource.Quantity `json:"memoryRequest,omitempty"`
+	CPURequest    *resource.Quantity `json:"cpuRequest,omitempty"`
 }
 
 // ServiceAccountIssuerDiscoveryConfig configures an OIDC Issuer.
diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go
index 4cbe872c3c..ca380afa66 100644
--- a/pkg/apis/kops/v1alpha2/cluster.go
+++ b/pkg/apis/kops/v1alpha2/cluster.go
@@ -261,10 +261,13 @@ type PodIdentityWebhookSpec struct {
 }
 
 type KarpenterConfig struct {
-	Enabled     bool   `json:"enabled,omitempty"`
-	LogEncoding string `json:"logEncoding,omitempty"`
-	LogLevel    string `json:"logLevel,omitempty"`
-	Image       string `json:"image,omitempty"`
+	Enabled       bool               `json:"enabled,omitempty"`
+	LogEncoding   string             `json:"logEncoding,omitempty"`
+	LogLevel      string             `json:"logLevel,omitempty"`
+	Image         string             `json:"image,omitempty"`
+	MemoryLimit   *resource.Quantity `json:"memoryLimit,omitempty"`
+	MemoryRequest *resource.Quantity `json:"memoryRequest,omitempty"`
+	CPURequest    *resource.Quantity `json:"cpuRequest,omitempty"`
 }
 
 // ServiceAccountIssuerDiscoveryConfig configures an OIDC Issuer.
diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go
index 595c3cdc18..24e64c6be4 100644
--- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go
+++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go
@@ -4605,6 +4605,9 @@ func autoConvert_v1alpha2_KarpenterConfig_To_kops_KarpenterConfig(in *KarpenterC
 	out.LogEncoding = in.LogEncoding
 	out.LogLevel = in.LogLevel
 	out.Image = in.Image
+	out.MemoryLimit = in.MemoryLimit
+	out.MemoryRequest = in.MemoryRequest
+	out.CPURequest = in.CPURequest
 	return nil
 }
 
@@ -4618,6 +4621,9 @@ func autoConvert_kops_KarpenterConfig_To_v1alpha2_KarpenterConfig(in *kops.Karpe
 	out.LogEncoding = in.LogEncoding
 	out.LogLevel = in.LogLevel
 	out.Image = in.Image
+	out.MemoryLimit = in.MemoryLimit
+	out.MemoryRequest = in.MemoryRequest
+	out.CPURequest = in.CPURequest
 	return nil
 }
 
diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
index e423dff845..ae2be4f308 100644
--- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
+++ b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
@@ -1368,7 +1368,7 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) {
 	if in.Karpenter != nil {
 		in, out := &in.Karpenter, &out.Karpenter
 		*out = new(KarpenterConfig)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	if in.PodIdentityWebhook != nil {
 		in, out := &in.PodIdentityWebhook, &out.PodIdentityWebhook
@@ -2735,6 +2735,21 @@ func (in *InstanceRequirementsSpec) DeepCopy() *InstanceRequirementsSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *KarpenterConfig) DeepCopyInto(out *KarpenterConfig) {
 	*out = *in
+	if in.MemoryLimit != nil {
+		in, out := &in.MemoryLimit, &out.MemoryLimit
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.MemoryRequest != nil {
+		in, out := &in.MemoryRequest, &out.MemoryRequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.CPURequest != nil {
+		in, out := &in.CPURequest, &out.CPURequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
 	return
 }
 
diff --git a/pkg/apis/kops/v1alpha3/cluster.go b/pkg/apis/kops/v1alpha3/cluster.go
index eb1aaf3f66..123a54f7a6 100644
--- a/pkg/apis/kops/v1alpha3/cluster.go
+++ b/pkg/apis/kops/v1alpha3/cluster.go
@@ -250,10 +250,13 @@ type ScalewaySpec struct {
 }
 
 type KarpenterConfig struct {
-	Enabled     bool   `json:"enabled,omitempty"`
-	LogEncoding string `json:"logEncoding,omitempty"`
-	LogLevel    string `json:"logLevel,omitempty"`
-	Image       string `json:"image,omitempty"`
+	Enabled       bool               `json:"enabled,omitempty"`
+	LogEncoding   string             `json:"logEncoding,omitempty"`
+	LogLevel      string             `json:"logLevel,omitempty"`
+	Image         string             `json:"image,omitempty"`
+	MemoryLimit   *resource.Quantity `json:"memoryLimit,omitempty"`
+	MemoryRequest *resource.Quantity `json:"memoryRequest,omitempty"`
+	CPURequest    *resource.Quantity `json:"cpuRequest,omitempty"`
 }
 
 // ServiceAccountIssuerDiscoveryConfig configures an OIDC Issuer.
diff --git a/pkg/apis/kops/v1alpha3/zz_generated.conversion.go b/pkg/apis/kops/v1alpha3/zz_generated.conversion.go
index 4132796459..5167e7b888 100644
--- a/pkg/apis/kops/v1alpha3/zz_generated.conversion.go
+++ b/pkg/apis/kops/v1alpha3/zz_generated.conversion.go
@@ -4998,6 +4998,9 @@ func autoConvert_v1alpha3_KarpenterConfig_To_kops_KarpenterConfig(in *KarpenterC
 	out.LogEncoding = in.LogEncoding
 	out.LogLevel = in.LogLevel
 	out.Image = in.Image
+	out.MemoryLimit = in.MemoryLimit
+	out.MemoryRequest = in.MemoryRequest
+	out.CPURequest = in.CPURequest
 	return nil
 }
 
@@ -5011,6 +5014,9 @@ func autoConvert_kops_KarpenterConfig_To_v1alpha3_KarpenterConfig(in *kops.Karpe
 	out.LogEncoding = in.LogEncoding
 	out.LogLevel = in.LogLevel
 	out.Image = in.Image
+	out.MemoryLimit = in.MemoryLimit
+	out.MemoryRequest = in.MemoryRequest
+	out.CPURequest = in.CPURequest
 	return nil
 }
 
diff --git a/pkg/apis/kops/v1alpha3/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha3/zz_generated.deepcopy.go
index 32fdaf53e2..a744653ab0 100644
--- a/pkg/apis/kops/v1alpha3/zz_generated.deepcopy.go
+++ b/pkg/apis/kops/v1alpha3/zz_generated.deepcopy.go
@@ -1263,7 +1263,7 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) {
 	if in.Karpenter != nil {
 		in, out := &in.Karpenter, &out.Karpenter
 		*out = new(KarpenterConfig)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }
@@ -2704,6 +2704,21 @@ func (in *InstanceRootVolumeSpec) DeepCopy() *InstanceRootVolumeSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *KarpenterConfig) DeepCopyInto(out *KarpenterConfig) {
 	*out = *in
+	if in.MemoryLimit != nil {
+		in, out := &in.MemoryLimit, &out.MemoryLimit
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.MemoryRequest != nil {
+		in, out := &in.MemoryRequest, &out.MemoryRequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.CPURequest != nil {
+		in, out := &in.CPURequest, &out.CPURequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
 	return
 }
 
diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go
index c7a91e28e8..a8f6c8766a 100644
--- a/pkg/apis/kops/zz_generated.deepcopy.go
+++ b/pkg/apis/kops/zz_generated.deepcopy.go
@@ -1360,7 +1360,7 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) {
 	if in.Karpenter != nil {
 		in, out := &in.Karpenter, &out.Karpenter
 		*out = new(KarpenterConfig)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }
@@ -2867,6 +2867,21 @@ func (in *InstanceRootVolumeSpec) DeepCopy() *InstanceRootVolumeSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *KarpenterConfig) DeepCopyInto(out *KarpenterConfig) {
 	*out = *in
+	if in.MemoryLimit != nil {
+		in, out := &in.MemoryLimit, &out.MemoryLimit
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.MemoryRequest != nil {
+		in, out := &in.MemoryRequest, &out.MemoryRequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
+	if in.CPURequest != nil {
+		in, out := &in.CPURequest, &out.CPURequest
+		x := (*in).DeepCopy()
+		*out = &x
+	}
 	return
 }
 
diff --git a/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template b/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template
index 5983ecf60e..3f172245fb 100644
--- a/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template
+++ b/upup/models/cloudup/resources/addons/karpenter.sh/k8s-1.19.yaml.template
@@ -1526,10 +1526,10 @@ spec:
               port: http
           resources:
             limits:
-              memory: 1Gi
+              memory: {{ or .Karpenter.MemoryLimit "1Gi" }}
             requests:
-              cpu: 500m
-              memory: 1Gi
+              cpu: {{ or .Karpenter.CPURequest "500m" }}
+              memory: {{ or .Karpenter.MemoryRequest "1Gi" }}
       nodeSelector: null
       # The template below patches the .Values.affinity to add a default label selector where not specificed
       affinity:


Test patch:
diff --git a/tests/integration/update_cluster/karpenter/data/aws_s3_object_cluster-completed.spec_content b/tests/integration/update_cluster/karpenter/data/aws_s3_object_cluster-completed.spec_content
index 086eab51ed..340e8392b1 100644
--- a/tests/integration/update_cluster/karpenter/data/aws_s3_object_cluster-completed.spec_content
+++ b/tests/integration/update_cluster/karpenter/data/aws_s3_object_cluster-completed.spec_content
@@ -55,10 +55,13 @@ spec:
     legacy: false
     useServiceAccountExternalPermissions: true
   karpenter:
+    cpuRequest: 100m
     enabled: true
     image: public.ecr.aws/karpenter/controller:v0.28.1
     logEncoding: console
     logLevel: debug
+    memoryLimit: 2Gi
+    memoryRequest: 500Mi
   keyStore: memfs://clusters.example.com/minimal.example.com/pki
   kubeAPIServer:
     allowPrivileged: true
diff --git a/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-bootstrap_content b/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-bootstrap_content
index 1e5a6a5603..e9fad92a81 100644
--- a/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-bootstrap_content
+++ b/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-bootstrap_content
@@ -120,7 +120,7 @@ spec:
     version: 9.99.0
   - id: k8s-1.19
     manifest: karpenter.sh/k8s-1.19.yaml
-    manifestHash: ff8aea6ec871d9b7d8482dab9831d002de4466243c9df7c10b363e47ccc58601
+    manifestHash: 83732936b11b5830020d8af7bf0955c4b6334c7a1ba93bf051b40bb79294075d
     name: karpenter.sh
     prune:
       kinds:
diff --git a/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-karpenter.sh-k8s-1.19_content b/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-karpenter.sh-k8s-1.19_content
index aa479f63b7..e992cff26f 100644
--- a/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-karpenter.sh-k8s-1.19_content
+++ b/tests/integration/update_cluster/karpenter/data/aws_s3_object_minimal.example.com-addons-karpenter.sh-k8s-1.19_content
@@ -1744,10 +1744,10 @@ spec:
           timeoutSeconds: 30
         resources:
           limits:
-            memory: 1Gi
+            memory: 2Gi
           requests:
-            cpu: 500m
-            memory: 1Gi
+            cpu: 100m
+            memory: 500Mi
         volumeMounts:
         - mountPath: /var/run/secrets/amazonaws.com/
           name: token-amazonaws-com
diff --git a/tests/integration/update_cluster/karpenter/in-v1alpha2.yaml b/tests/integration/update_cluster/karpenter/in-v1alpha2.yaml
index 9c9314f1a4..c21d91061c 100644
--- a/tests/integration/update_cluster/karpenter/in-v1alpha2.yaml
+++ b/tests/integration/update_cluster/karpenter/in-v1alpha2.yaml
@@ -21,7 +21,10 @@ spec:
   iam:
     useServiceAccountExternalPermissions: true
   karpenter:
+    cpuRequest: 100m
     enabled: true
+    memoryRequest: 500Mi
+    memoryLimit: 2Gi
   kubelet:
     anonymousAuth: false
   kubernetesVersion: v1.25.0