openshift / instaslice-operator

InstaSlice Operator facilitates slicing of accelerators using stable APIs
Apache License 2.0
9 stars 12 forks source link

Pod Annotator only sets the first container in the list and ignores init containers. #25

Open kannon92 opened 2 months ago

kannon92 commented 2 months ago

The code below patches the containers of the first pod. There are two bugs and one question I see here:

1) An init container can have access to a GPU so one may not be able to get the MIG slice for the init container.

2) Pods can take a list of containers and this only edits the first container in the list.

Question: Ephemeral containers are also ignored but not sure if they need access to the MIG slice.

func (a *PodAnnotator) Handle(ctx context.Context, req admission.Request) admission.Response {

... pod.Spec.Containers[0].EnvFrom = append(pod.Spec.Containers[0].EnvFrom, v1.EnvFromSource{ ConfigMapRef: &v1.ConfigMapEnvSource{ LocalObjectReference: v1.LocalObjectReference{Name: configMapName}, }, })

// Add extended resource to resource limits
if pod.Spec.Containers[0].Resources.Limits == nil {
    pod.Spec.Containers[0].Resources.Limits = make(v1.ResourceList)
}
pod.Spec.Containers[0].Resources.Limits[v1.ResourceName(extendedResourceName)] = resource.MustParse("1")

// Marshal the updated pod object back to JSON
marshaledPod, err := json.Marshal(pod)
if err != nil {
    return admission.Errored(500, fmt.Errorf("could not marshal pod: %v", err))
}

// Return the patch response
return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)

}

asm582 commented 2 months ago

Thanks for bringing this up, this is by design.

asm582 commented 4 weeks ago

This came up in a different PR review, comment: https://github.com/openshift/instaslice-operator/pull/141/files/b36341967d2a863d38d08993917af4d8ecc0aa75#r1792193024