Container deletion from the workload #57
Conversation
pkg/podstate/podstate.go
Outdated
| return powerv1.GuaranteedPod{} | ||
| } | ||
|
|
||
| func (s *State) GetPodFromStateUID(podUID string) powerv1.GuaranteedPod { |
There was a problem hiding this comment.
Kubernetes already enforces that a pod name/namespace combination is unique so why is the existing GetPodFromState function not sufficient?
There was a problem hiding this comment.
Hi @bartwensley,
The implementation of the validation through the podUID, was proposed to be visually compatible with the proposal of using container UID. In this case, all functions used to remove or identify pods and containers would use the same parameters.
There was a problem hiding this comment.
IMO this part of the change is unnecessary and adds complexity by introducing a second way to retrieve the pod.
There was a problem hiding this comment.
I fixed these modifications per your suggestion.
| logger.V(5).Info("checking if there are new containers for the workload") | ||
| for _, container := range nodeContainers { | ||
| if !isContainerInList(container.Name, podStateContainers, logger) { | ||
| if !isContainerInList(container.Name, container.Id, podStateContainers, logger) { |
There was a problem hiding this comment.
Would be nice to create a new unit test for the Reconcile function that would fail on the original code and pass with your proposed changes.
When deleting pods that use the resources of a given power profile, the workload may be incorrectly updated, removing all existing container entries. This error can be observed whenever containers with the same name, but different pods, share resources of the same workload. This commit changes the container identification of a workload, searching for its UID instead of its name, as well as including workload information when admitting a powerpod. Signed-off-by: Eduardo Juliano Alberti <[email protected]>
ejalberti
force-pushed
the
deleting-pod-workload
branch
from
9d99832 to
64f823a
Compare
2 participants
When deleting pods that use the resources of a given power profile, the workload may be incorrectly updated, removing all existing container entries. This error can be observed whenever containers with the same name, but different pods, share resources of the same workload.
This commit changes the container identification of a workload, searching for its UID instead of its name, as well as including workload information when admitting a powerpod.