Repository: influxdata/telegraf-operator (go)

Issue:
Allow removing resource requests/limits on sidecar
In my case I'd like to remove the CPU resource limit on the telegraf sidecar. If I leave out the annotation, it will use the default value. It would be nice if there are either 1) no resource requests/limits on the sidecar, or 2) a way to disable setting a default value



Pull request:
feat: allow disabling telegraf resources by providing an empty string
Closes #84

Allows disabling resource requests and/or limits for telegraf sidecars by providing an empty string as value.

- [x] Signed the [InfluxData CLA](https://www.influxdata.com/legal/cla/)
  ([if necessary](https://github.com/influxdata/telegraf-operator/blob/master/CONTRIBUTING.md#sign-the-influxdata-cla))
- [x] Tests pass (no build errors)
- [x] Rebased/mergeable


Patch:
diff --git a/sidecar.go b/sidecar.go
index 3fb148c..8c9bf75 100644
--- a/sidecar.go
+++ b/sidecar.go
@@ -134,18 +134,14 @@ func (h *sidecarHandler) shouldAddIstioTelegrafSidecar(pod *corev1.Pod) bool {
 	return false
 }
 
-func (h *sidecarHandler) validateRequestsAndLimits() error {
-	if _, err := resource.ParseQuantity(h.RequestsCPU); err != nil {
-		return err
-	}
-	if _, err := resource.ParseQuantity(h.RequestsMemory); err != nil {
-		return err
-	}
-	if _, err := resource.ParseQuantity(h.LimitsCPU); err != nil {
-		return err
-	}
-	if _, err := resource.ParseQuantity(h.LimitsMemory); err != nil {
-		return err
+func (h *sidecarHandler) validateRequestsAndLimits() (err error) {
+	for _, value := range []string{h.RequestsCPU, h.RequestsMemory, h.LimitsCPU, h.LimitsMemory} {
+		if value != "" {
+			_, err = resource.ParseQuantity(value)
+			if err != nil {
+				return
+			}
+		}
 	}
 
 	return nil
@@ -344,14 +340,29 @@ func (h *sidecarHandler) newVolume(name, containerName string) corev1.Volume {
 	}
 }
 
-// parseCustomOrDefaultQuantity parses custom quantity from annotations,
+// parseCustomOrDefaultQuantity parses custom quantity from annotations, storing it in provided ResourceList as resourceName,
 // defaulting to quantity specified to the handler if the custom one is not valid
-func (h *sidecarHandler) parseCustomOrDefaultQuantity(customQuantity string, defaultQuantity string) (quantity resource.Quantity, err error) {
+func (h *sidecarHandler) parseCustomOrDefaultQuantity(result corev1.ResourceList, resourceName corev1.ResourceName, customQuantity string, defaultQuantity string) (err error) {
+	// if default or current value is an empty string, do not set value
+	if customQuantity == "" {
+		return nil
+	}
+	var quantity resource.Quantity
 	if quantity, err = resource.ParseQuantity(customQuantity); err != nil {
 		h.Logger.Info(fmt.Sprintf("unable to parse resource \"%s\": %v", customQuantity, err))
+
+		if defaultQuantity == "" {
+			return nil
+		}
 		quantity, err = resource.ParseQuantity(defaultQuantity)
 	}
-	return quantity, err
+
+	if err != nil {
+		return err
+	}
+
+	result[resourceName] = quantity
+	return nil
 }
 
 func (h *sidecarHandler) newContainer(pod *corev1.Pod, containerName string) (corev1.Container, error) {
@@ -387,23 +398,20 @@ func (h *sidecarHandler) newContainer(pod *corev1.Pod, containerName string) (co
 		telegrafLimitsMemory = h.LimitsMemory
 	}
 
-	var parsedRequestsCPU resource.Quantity
-	var parsedRequestsMemory resource.Quantity
-	var parsedLimitsCPU resource.Quantity
-	var parsedLimitsMemory resource.Quantity
-	var err error
+	resourceRequests := corev1.ResourceList{}
+	resourceLimits := corev1.ResourceList{}
 
-	if parsedRequestsCPU, err = h.parseCustomOrDefaultQuantity(telegrafRequestsCPU, h.RequestsCPU); err != nil {
+	if err := h.parseCustomOrDefaultQuantity(resourceRequests, "cpu", telegrafRequestsCPU, h.RequestsCPU); err != nil {
 		return corev1.Container{}, err
 	}
-	if parsedRequestsMemory, err = h.parseCustomOrDefaultQuantity(telegrafRequestsMemory, h.RequestsMemory); err != nil {
+	if err := h.parseCustomOrDefaultQuantity(resourceRequests, "memory", telegrafRequestsMemory, h.RequestsMemory); err != nil {
 		return corev1.Container{}, err
 	}
 
-	if parsedLimitsCPU, err = h.parseCustomOrDefaultQuantity(telegrafLimitsCPU, h.LimitsCPU); err != nil {
+	if err := h.parseCustomOrDefaultQuantity(resourceLimits, "cpu", telegrafLimitsCPU, h.LimitsCPU); err != nil {
 		return corev1.Container{}, err
 	}
-	if parsedLimitsMemory, err = h.parseCustomOrDefaultQuantity(telegrafLimitsMemory, h.LimitsMemory); err != nil {
+	if err := h.parseCustomOrDefaultQuantity(resourceLimits, "memory", telegrafLimitsMemory, h.LimitsMemory); err != nil {
 		return corev1.Container{}, err
 	}
 
@@ -414,14 +422,8 @@ func (h *sidecarHandler) newContainer(pod *corev1.Pod, containerName string) (co
 		Image:   telegrafImage,
 		Command: telegrafContainerCommand,
 		Resources: corev1.ResourceRequirements{
-			Limits: corev1.ResourceList{
-				"cpu":    parsedLimitsCPU,
-				"memory": parsedLimitsMemory,
-			},
-			Requests: corev1.ResourceList{
-				"cpu":    parsedRequestsCPU,
-				"memory": parsedRequestsMemory,
-			},
+			Requests: resourceRequests,
+			Limits:   resourceLimits,
 		},
 		Env: []corev1.EnvVar{
 			{


Test patch:
diff --git a/sidecar_test.go b/sidecar_test.go
index af82181..2a1e9f2 100644
--- a/sidecar_test.go
+++ b/sidecar_test.go
@@ -132,6 +132,16 @@ func Test_validateRequestsAndLimits(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "validate empty values are accepted",
+			sidecar: &sidecarHandler{
+				RequestsCPU:    "",
+				RequestsMemory: "",
+				LimitsCPU:      "",
+				LimitsMemory:   "",
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -1188,6 +1198,53 @@ spec:
   - name: telegraf-config
     secret:
       secretName: telegraf-config-myname
+status: {}
+      `,
+			wantSecrets: []string{testEmptySecret},
+		},
+		{
+			name: "validate empty resource requests",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						TelegrafClass:          "default",
+						TelegrafRequestsCPU:    "",
+						TelegrafRequestsMemory: "",
+						TelegrafLimitsCPU:      "",
+						TelegrafLimitsMemory:   "",
+					},
+				},
+			},
+			wantPod: `
+metadata:
+  annotations:
+    telegraf.influxdata.com/class: default
+    telegraf.influxdata.com/limits-cpu: ""
+    telegraf.influxdata.com/limits-memory: ""
+    telegraf.influxdata.com/requests-cpu: ""
+    telegraf.influxdata.com/requests-memory: ""
+  creationTimestamp: null
+spec:
+  containers:
+  - command:
+    - telegraf
+    - --config
+    - /etc/telegraf/telegraf.conf
+    env:
+    - name: NODENAME
+      valueFrom:
+        fieldRef:
+          fieldPath: spec.nodeName
+    image: docker.io/library/telegraf:1.19
+    name: telegraf
+    resources: {}
+    volumeMounts:
+    - mountPath: /etc/telegraf
+      name: telegraf-config
+  volumes:
+  - name: telegraf-config
+    secret:
+      secretName: telegraf-config-myname
 status: {}
       `,
 			wantSecrets: []string{testEmptySecret},