Skip to content

Commit

Permalink
Fix issue with node that has no non-DS pods not getting deleted
Browse files Browse the repository at this point in the history
  • Loading branch information
TwiN committed May 8, 2020
1 parent b1f4e9a commit dfe78b1
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
6 changes: 1 addition & 5 deletions k8s/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import (
)

func CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(kubernetesClient KubernetesClientApi, oldNode *v1.Node, targetNodes []*v1.Node) bool {
// If there's no target nodes, then there's definitely not enough resources available
if len(targetNodes) == 0 {
return false
}
totalAvailableTargetCpu := int64(0)
totalAvailableTargetMemory := int64(0)
// Get resources available in target nodes
Expand Down Expand Up @@ -69,7 +65,7 @@ func CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(kubernetesClient Kube
}
leftOverCpu := totalAvailableTargetCpu - cpuNeeded
leftOverMemory := totalAvailableTargetMemory - memoryNeeded
return leftOverCpu > 0 && leftOverMemory > 0
return leftOverCpu >= 0 && leftOverMemory >= 0
}

func AnnotateNodeByHostName(kubernetesClient KubernetesClientApi, hostName, key, value string) error {
Expand Down
12 changes: 10 additions & 2 deletions k8s/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,15 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withNoTargetNodes
if hasEnoughResources {
t.Error("there's no target nodes; there definitely shouldn't have been enough space")
}
if mockKubernetesClient.Counter["GetPodsInNode"] != 0 {
t.Error("GetPodInNode shouldn't have been called")
}

func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withNoTargetNodesButOldNodeOnlyHasPodsFromDaemonSets(t *testing.T) {
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
oldNodePod := k8stest.CreateTestPod("old-node-pod-1", oldNode.Name, "500Mi", "500Mi", true)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode}, []v1.Pod{oldNodePod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{})
if !hasEnoughResources {
t.Error("there's no target nodes, but the only pods in the old node are from daemon sets")
}
}

0 comments on commit dfe78b1

Please sign in to comment.