update mutator and error message

opendatahub-io · Jul 11, 2023 · 60bcc0f · 60bcc0f
1 parent a6c1c56
commit 60bcc0f
Show file tree

Hide file tree

Showing 8 changed files with 13 additions and 11 deletions.
diff --git a/pkg/webhook/admission/pod/accelerator_injector.go b/pkg/webhook/admission/pod/accelerator_injector.go
@@ -28,7 +28,7 @@ const (
 	NvidiaGPUTaintValue        = "present"
 )
 
-func InjectGKEAcceleratorSelector(pod *v1.Pod, _ *v1.Namespace) error {
+func InjectGKEAcceleratorSelector(pod *v1.Pod) error {
 	gpuEnabled := false
 	for _, container := range pod.Spec.Containers {
 		if _, ok := container.Resources.Limits[constants.NvidiaGPUResourceType]; ok {

diff --git a/pkg/webhook/admission/pod/accelerator_injector_test.go b/pkg/webhook/admission/pod/accelerator_injector_test.go
@@ -94,7 +94,7 @@ func TestAcceleratorInjector(t *testing.T) {
 	}
 
 	for name, scenario := range scenarios {
-		InjectGKEAcceleratorSelector(scenario.original, nil)
+		InjectGKEAcceleratorSelector(scenario.original)
 		// cmd.Diff complains on ResourceList when Nvidia is key. Objects are explicitly compared
 		if diff := cmp.Diff(
 			scenario.expected.Spec.NodeSelector,

diff --git a/pkg/webhook/admission/pod/agent_injector.go b/pkg/webhook/admission/pod/agent_injector.go
@@ -116,7 +116,7 @@ func getLoggerConfigs(configMap *v1.ConfigMap) (*LoggerConfig, error) {
 	return loggerConfig, nil
 }
 
-func (ag *AgentInjector) InjectAgent(pod *v1.Pod, _ *v1.Namespace) error {
+func (ag *AgentInjector) InjectAgent(pod *v1.Pod) error {
 	// Only inject the model agent sidecar if the required annotations are set
 	_, injectLogger := pod.ObjectMeta.Annotations[constants.LoggerInternalAnnotationKey]
 	_, injectPuller := pod.ObjectMeta.Annotations[constants.AgentShouldInjectAnnotationKey]

diff --git a/pkg/webhook/admission/pod/agent_injector_test.go b/pkg/webhook/admission/pod/agent_injector_test.go
@@ -1112,7 +1112,7 @@ func TestAgentInjector(t *testing.T) {
 			loggerConfig,
 			batcherTestConfig,
 		}
-		injector.InjectAgent(scenario.original, nil)
+		injector.InjectAgent(scenario.original)
 		if diff, _ := kmp.SafeDiff(scenario.expected.Spec, scenario.original.Spec); diff != "" {
 			t.Errorf("Test %q unexpected result (-want +got): %v", name, diff)
 		}

diff --git a/pkg/webhook/admission/pod/metrics_aggregate_injector.go b/pkg/webhook/admission/pod/metrics_aggregate_injector.go
@@ -76,7 +76,7 @@ func setMetricAggregationEnvVars(pod *v1.Pod) {
 
 // InjectMetricsAggregator looks for the annotations to enable aggregate kserve-container and queue-proxy metrics and
 // if specified, sets port-related EnvVars in queue-proxy and the aggregate prometheus annotation.
-func (ma *MetricsAggregator) InjectMetricsAggregator(pod *v1.Pod, _ *v1.Namespace) error {
+func (ma *MetricsAggregator) InjectMetricsAggregator(pod *v1.Pod) error {
 	//Only set metric configs if the required annotations are set
 	enableMetricAggregation, ok := pod.ObjectMeta.Annotations[constants.EnableMetricAggregation]
 	if !ok {

diff --git a/pkg/webhook/admission/pod/metrics_aggregate_injector_test.go b/pkg/webhook/admission/pod/metrics_aggregate_injector_test.go
@@ -290,7 +290,7 @@ func TestInjectMetricsAggregator(t *testing.T) {
 	}
 
 	for name, scenario := range scenarios {
-		ma.InjectMetricsAggregator(scenario.original, nil)
+		ma.InjectMetricsAggregator(scenario.original)
 		if diff, _ := kmp.SafeDiff(scenario.expected.Spec, scenario.original.Spec); diff != "" {
 			t.Errorf("Test %q unexpected result (-want +got): %v", name, diff)
 		}

diff --git a/pkg/webhook/admission/pod/mutator.go b/pkg/webhook/admission/pod/mutator.go
@@ -123,15 +123,17 @@ func (mutator *Mutator) mutate(pod *v1.Pod, configMap *v1.ConfigMap, targetNs *v
 		return err
 	}
 
-	mutators := []func(pod *v1.Pod, targetNs *v1.Namespace) error{
+	mutators := []func(pod *v1.Pod) error{
 		InjectGKEAcceleratorSelector,
-		storageInitializer.InjectStorageInitializer,
+		func(pod *v1.Pod) error {
+			return storageInitializer.InjectStorageInitializer(pod, targetNs)
+		},
 		agentInjector.InjectAgent,
 		metricsAggregator.InjectMetricsAggregator,
 	}
 
 	for _, mutator := range mutators {
-		if err := mutator(pod, targetNs); err != nil {
+		if err := mutator(pod); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/webhook/admission/pod/storage_initializer_injector.go b/pkg/webhook/admission/pod/storage_initializer_injector.go
@@ -342,15 +342,15 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod, targ
 	}
 	uidStr := targetNs.Annotations[OpenShiftUidRangeAnnotationKey]
 	if uidStr == "" {
-		return fmt.Errorf("could not find OpenShift internal annotation %s on namespace: %s", OpenShiftUidRangeAnnotationKey, targetNs.Name)
+		return fmt.Errorf("could not find OpenShift internal annotation %s for calculating the process UID (minRange + 1) on namespace: %s", OpenShiftUidRangeAnnotationKey, targetNs.Name)
 	}
 	uidStrParts := strings.Split(uidStr, "/")
 	if uid, err := strconv.ParseInt(uidStrParts[0], 10, 64); err == nil {
 		// Set the uid to the first uid in the namespaces range + 1
 		uid++
 		initContainer.SecurityContext.RunAsUser = ptr.Int64(uid)
 	} else {
-		return fmt.Errorf("could not parse value %s in annotation %s on namespace: %s", uidStr, OpenShiftUidRangeAnnotationKey, targetNs.Name)
+		return fmt.Errorf("could not parse value %s in annotation %s as an int64 value on namespace: %s", uidStr, OpenShiftUidRangeAnnotationKey, targetNs.Name)
 	}
 
 	// Add init container to the spec