feat: add native support for pod eviction #3554
Conversation
y-rabie
force-pushed
the
add-pod-eviction
branch
2 times, most recently
from
e3ac2db to
f84fb57
Compare
| } | ||
|
|
||
| if err := evictable.Evict(context.Background(), path); err != nil { | ||
| if strings.Contains(err.Error(), "violate") { |
There was a problem hiding this comment.
It would be best if Evict returns a sentinel error that we can test against via errors.Is vs looking for a string
| if err := evictable.Evict(context.Background(), path); err != nil { | ||
| if strings.Contains(err.Error(), "violate") { | ||
| // If the error is due to PDB violation, don't attempt to evict the rest of the selection. | ||
| pdbViolation = true |
There was a problem hiding this comment.
This seems a bit inconsistent. If a pod eviction failed on pdb we don't process the rest of the batch. But if there is any other eviction issue we continue. I think we should be consistent here i.e we either stop processing the batch or do our best to continue processing and report back which evictions failed and why.
I also think this should be confirmed pod evictions could yield service disruption which could break workloads especially if we fail to reschedule...
There was a problem hiding this comment.
This seems a bit inconsistent. If a pod eviction failed on pdb we don't process the rest of the batch. But if there is any other eviction issue we continue. I think we should be consistent here i.e we either stop processing the batch or do our best to continue processing and report back which evictions failed and why.
It just seemed reasonable that for PDB violation error, we need to stop, since we know the rest will fail as well. For other errors, I accounted for the fact that the error might be transient, and so other pods may succeed eviction.
I'm okay however with being consistent, and I think not trying the rest of the batch is best.
y-rabie
force-pushed
the
add-pod-eviction
branch
from
f84fb57 to
72d29b3
Compare
Signed-off-by: y-rabie <youssef.rabie@procore.com>
y-rabie
force-pushed
the
add-pod-eviction
branch
from
72d29b3 to
e078681
Compare
2 participants
This adds a new key binding
Ctrl+vfor pod eviction. I think it's useful to have that functionality as a form of less dangerous deletion in restricted environments.There is no kubectl command for it, but there are some plugins arounds such as: (https://github.com/rajatjindal/kubectl-evict-pod and https://github.com/ueokande/kubectl-evict). Using one of these kubectl plugins within a k9s binding plugin however won't make multi-selection available for eviction, and the user will be restricted to evict pod one by one.
Since this is a somewhat safe operation, I've opted not to have a confirmation dialog for it similar to kill. But I'm open to having it.