Set resource request to use default when limit annotation is zero.

pkg/webhook/mutatingwebhook.go

@@ -123,6 +123,34 @@ func (si *SidecarInjector) Handle(_ context.Context, req admission.Request) admi
 	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
 }
 
+// populateResource assigns request and limits based on the following conditions:
+//  1. If both of the request and limit are unset, assign the default values.
+//  2. If one of the request or limit is set and another is unset, enforce them to be set as the same.
+//
+// Note: when the annotation limit is zero and request is unset, we set request to use the default value.
+func populateResource(requestQuantity, limitQuantity *resource.Quantity, defaultRequestQuantity, defaultLimitQuantity resource.Quantity) {
+	// Use defaults when no annotations are set.
+	if requestQuantity.Format == "" && limitQuantity.Format == "" {
+		*requestQuantity = defaultRequestQuantity
+		*limitQuantity = defaultLimitQuantity
+	}
+
+	// Set request to equal default when limit is zero/unlimited and request is unset.
+	if limitQuantity.IsZero() && requestQuantity.Format == "" {
+		*requestQuantity = defaultRequestQuantity
+	}
+
+	// Set request to equal limit when request annotation is not provided.
+	if requestQuantity.Format == "" {
+		*requestQuantity = *limitQuantity
+	}
+
+	// Set limit to equal request when limit annotation is not provided.
+	if limitQuantity.Format == "" {
+		*limitQuantity = *requestQuantity
+	}
+}
+
 // use the default config values,
 // overwritten by the user input from pod annotations.
 func (si *SidecarInjector) prepareConfig(annotations map[string]string) (*Config, error) {
@@ -140,23 +168,6 @@ func (si *SidecarInjector) prepareConfig(annotations map[string]string) (*Config
 		return nil, fmt.Errorf("failed to parse sidecar container resource allocation from pod annotations: %w", err)
 	}
 
-	// if both of the request and limit are unset, assign the default values.
-	// if one of the request or limit is set and another is unset, enforce them to be set as the same.
-	populateResource := func(rq, lq *resource.Quantity, drq, dlq resource.Quantity) {
-		if rq.Format == "" && lq.Format == "" {
-			*rq = drq
-			*lq = dlq
-		}
-
-		if lq.Format == "" {
-			*lq = *rq
-		}
-
-		if rq.Format == "" {
-			*rq = *lq
-		}
-	}
-
 	populateResource(&config.CPURequest, &config.CPULimit, si.Config.CPURequest, si.Config.CPULimit)
 	populateResource(&config.MemoryRequest, &config.MemoryLimit, si.Config.MemoryRequest, si.Config.MemoryLimit)
 	populateResource(&config.EphemeralStorageRequest, &config.EphemeralStorageLimit, si.Config.EphemeralStorageRequest, si.Config.EphemeralStorageLimit)

pkg/webhook/mutatingwebhook_test.go

@@ -121,11 +121,11 @@ func TestPrepareConfig(t *testing.T) {
 				ContainerImage:          FakeConfig().ContainerImage,
 				ImagePullPolicy:         FakeConfig().ImagePullPolicy,
 				CPULimit:                resource.Quantity{},
-				CPURequest:              resource.Quantity{},
+				CPURequest:              FakeConfig().CPURequest,
 				MemoryLimit:             resource.Quantity{},
-				MemoryRequest:           resource.Quantity{},
+				MemoryRequest:           FakeConfig().MemoryRequest,
 				EphemeralStorageLimit:   resource.Quantity{},
-				EphemeralStorageRequest: resource.Quantity{},
+				EphemeralStorageRequest: FakeConfig().EphemeralStorageRequest,
 			},
 			expectErr: false,
 		},
@@ -229,20 +229,19 @@ func TestPrepareConfig(t *testing.T) {
 		},
 	}
 
-	for n, tc := range testCases {
-		t.Logf("test case %v: %s", n+1, tc.name)
+	for _, tc := range testCases {
 		si := SidecarInjector{
 			Client:  nil,
 			Config:  FakeConfig(),
 			Decoder: admission.NewDecoder(runtime.NewScheme()),
 		}
 		gotConfig, gotErr := si.prepareConfig(tc.annotations)
 		if tc.expectErr != (gotErr != nil) {
-			t.Errorf("Expect error: %v, but got error: %v", tc.expectErr, gotErr)
+			t.Errorf(`for "%s", expect error: %v, but got error: %v`, tc.name, tc.expectErr, gotErr)
 		}
 
 		if diff := cmp.Diff(gotConfig, tc.wantConfig); diff != "" {
-			t.Errorf("config differ (-got, +want)\n%s", diff)
+			t.Errorf(`for "%s", config differ (-got, +want)\n%s`, tc.name, diff)
 		}
 	}
 }