Skip to content

Commit

Permalink
Closes #1: Filter out pods from daemon sets when calculating resources
Browse files Browse the repository at this point in the history
  • Loading branch information
@TwiN
TwiN committed May 8, 2020
1 parent b3e9912 commit 2c85128
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 50 deletions.
11 changes: 11 additions & 0 deletions k8s/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ func CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(kubernetesClient Kube
return true
}
for _, podInNode := range podsInNode {
// Ignore DaemonSets in the old node, because these pods will also be present in the target nodes
hasDaemonSetOwnerReference := false
for _, owner := range podInNode.GetOwnerReferences() {
if owner.Kind == "DaemonSet" {
hasDaemonSetOwnerReference = true
break
}
}
if hasDaemonSetOwnerReference {
continue
}
for _, container := range podInNode.Spec.Containers {
if container.Resources.Requests.Cpu() != nil {
// Subtract the cpu request of the pod from the node's total allocatable
Expand Down
32 changes: 16 additions & 16 deletions k8s/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(t *testing.T) {
// space in the target nodes)
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
newNode := k8stest.CreateTestNode("new-node-1", "1000m", "1000Mi")
oldNodePod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "100m", "100Mi")
oldNodePod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "100m", "100Mi", false)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode, newNode}, []v1.Pod{oldNodePod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{&newNode})
Expand All @@ -28,8 +28,8 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(t *testing.T) {
func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_whenNotEnoughSpaceInNewNodes(t *testing.T) {
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
newNode := k8stest.CreateTestNode("new-node-1", "1000m", "1000Mi")
oldNodePod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "200m", "200Mi")
newNodePod := k8stest.CreateTestPod("new-pod-1", newNode.Name, "900m", "200Mi")
oldNodePod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "200m", "200Mi", false)
newNodePod := k8stest.CreateTestPod("new-pod-1", newNode.Name, "900m", "200Mi", false)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode, newNode}, []v1.Pod{oldNodePod, newNodePod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{&newNode})
Expand All @@ -44,10 +44,10 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_whenNotEnoughSpac
func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withMultiplePods(t *testing.T) {
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
newNode := k8stest.CreateTestNode("new-node-1", "1000m", "1000Mi")
oldNodeFirstPod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "300m", "0")
oldNodeSecondPod := k8stest.CreateTestPod("old-pod-2", oldNode.Name, "300m", "0")
oldNodeThirdPod := k8stest.CreateTestPod("old-pod-3", oldNode.Name, "300m", "0")
newNodePod := k8stest.CreateTestPod("new-pod-1", newNode.Name, "200m", "200Mi")
oldNodeFirstPod := k8stest.CreateTestPod("old-pod-1", oldNode.Name, "300m", "0", false)
oldNodeSecondPod := k8stest.CreateTestPod("old-pod-2", oldNode.Name, "300m", "0", false)
oldNodeThirdPod := k8stest.CreateTestPod("old-pod-3", oldNode.Name, "300m", "0", false)
newNodePod := k8stest.CreateTestPod("new-pod-1", newNode.Name, "200m", "200Mi", false)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode, newNode}, []v1.Pod{oldNodeFirstPod, oldNodeSecondPod, oldNodeThirdPod, newNodePod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{&newNode})
Expand All @@ -63,9 +63,9 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withMultipleTarge
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
firstNewNode := k8stest.CreateTestNode("new-node-1", "1000m", "1000Mi")
secondNewNode := k8stest.CreateTestNode("new-node-2", "1000m", "1000Mi")
oldNodeFirstPod := k8stest.CreateTestPod("old-node-pod-1", oldNode.Name, "500m", "0")
oldNodeSecondPod := k8stest.CreateTestPod("old-node-pod-2", oldNode.Name, "500m", "0")
oldNodeThirdPod := k8stest.CreateTestPod("old-node-pod-3", oldNode.Name, "500m", "0")
oldNodeFirstPod := k8stest.CreateTestPod("old-node-pod-1", oldNode.Name, "500m", "0", false)
oldNodeSecondPod := k8stest.CreateTestPod("old-node-pod-2", oldNode.Name, "500m", "0", false)
oldNodeThirdPod := k8stest.CreateTestPod("old-node-pod-3", oldNode.Name, "500m", "0", false)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode, firstNewNode, secondNewNode}, []v1.Pod{oldNodeFirstPod, oldNodeSecondPod, oldNodeThirdPod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{&firstNewNode, &secondNewNode})
Expand All @@ -81,11 +81,11 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withPodsSpreadAcr
oldNode := k8stest.CreateTestNode("old-node", "0m", "0m")
firstNewNode := k8stest.CreateTestNode("new-node-1", "1000m", "1000Mi")
secondNewNode := k8stest.CreateTestNode("new-node-2", "1000m", "1000Mi")
firstNewNodePod := k8stest.CreateTestPod("new-node-1-pod-1", oldNode.Name, "0", "300Mi")
secondNewNodePod := k8stest.CreateTestPod("new-node-2-pod-1", oldNode.Name, "0", "300Mi")
oldNodeFirstPod := k8stest.CreateTestPod("old-node-pod-1", oldNode.Name, "0", "500Mi")
oldNodeSecondPod := k8stest.CreateTestPod("old-node-pod-2", oldNode.Name, "0", "500Mi")
oldNodeThirdPod := k8stest.CreateTestPod("old-node-pod-3", oldNode.Name, "0", "500Mi")
firstNewNodePod := k8stest.CreateTestPod("new-node-1-pod-1", oldNode.Name, "0", "300Mi", false)
secondNewNodePod := k8stest.CreateTestPod("new-node-2-pod-1", oldNode.Name, "0", "300Mi", false)
oldNodeFirstPod := k8stest.CreateTestPod("old-node-pod-1", oldNode.Name, "0", "500Mi", false)
oldNodeSecondPod := k8stest.CreateTestPod("old-node-pod-2", oldNode.Name, "0", "500Mi", false)
oldNodeThirdPod := k8stest.CreateTestPod("old-node-pod-3", oldNode.Name, "0", "500Mi", false)
mockKubernetesClient := k8stest.NewMockKubernetesClient([]v1.Node{oldNode, firstNewNode, secondNewNode}, []v1.Pod{oldNodeFirstPod, oldNodeSecondPod, oldNodeThirdPod, firstNewNodePod, secondNewNodePod})

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{&firstNewNode, &secondNewNode})
Expand All @@ -99,7 +99,7 @@ func TestCheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes_withPodsSpreadAcr

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

hasEnoughResources := CheckIfNodeHasEnoughResourcesToTransferAllPodsInNodes(mockKubernetesClient, &oldNode, []*v1.Node{})
Expand Down
8 changes: 7 additions & 1 deletion k8stest/k8stest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

type MockKubernetesClient struct {
Expand Down Expand Up @@ -84,7 +85,7 @@ func CreateTestNode(name string, allocatableCpu, allocatableMemory string) v1.No
return node
}

func CreateTestPod(name string, nodeName, cpuRequest, cpuMemory string) v1.Pod {
func CreateTestPod(name, nodeName, cpuRequest, cpuMemory string, isDaemonSet bool) v1.Pod {
pod := v1.Pod{
Spec: v1.PodSpec{
NodeName: nodeName,
Expand All @@ -100,5 +101,10 @@ func CreateTestPod(name string, nodeName, cpuRequest, cpuMemory string) v1.Pod {
},
}
pod.SetName(name)
if isDaemonSet {
pod.SetOwnerReferences([]metav1.OwnerReference{{Kind: "DaemonSet"}})
} else {
pod.SetOwnerReferences([]metav1.OwnerReference{{Kind: "ReplicaSet"}})
}
return pod
}
12 changes: 4 additions & 8 deletions main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ func HandleRollingUpgrade(kubernetesClient k8s.KubernetesClientApi, ec2Service e
for _, autoScalingGroup := range autoScalingGroups {
outdatedInstances, updatedInstances, err := SeparateOutdatedFromUpdatedInstances(autoScalingGroup, ec2Service)
if err != nil {
log.Printf("[%s] Unable to separate outdated instances from updated instances: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), err.Error())
log.Printf("[%s] Skipping", aws.StringValue(autoScalingGroup.AutoScalingGroupName))
log.Printf("[%s] Skipping because unable to separate outdated instances from updated instances: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), err.Error())
continue
}
if config.Get().Debug {
Expand Down Expand Up @@ -94,8 +93,7 @@ func HandleRollingUpgrade(kubernetesClient k8s.KubernetesClientApi, ec2Service e
for _, outdatedInstance := range outdatedInstances {
node, err := kubernetesClient.GetNodeByHostName(aws.StringValue(outdatedInstance.InstanceId))
if err != nil {
log.Printf("[%s][%s] Unable to get outdated node from Kubernetes: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
log.Printf("[%s][%s] Skipping", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId))
log.Printf("[%s][%s] Skipping because unable to get outdated node from Kubernetes: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
continue
}

Expand All @@ -107,8 +105,7 @@ func HandleRollingUpgrade(kubernetesClient k8s.KubernetesClientApi, ec2Service e
// Annotate the node to persist the fact that the rolling update process has begun
err := k8s.AnnotateNodeByHostName(kubernetesClient, aws.StringValue(outdatedInstance.InstanceId), k8s.RollingUpdateStartedTimestampAnnotationKey, time.Now().Format(time.RFC3339))
if err != nil {
log.Printf("[%s][%s] Unable to annotate node: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
log.Printf("[%s][%s] Skipping", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId))
log.Printf("[%s][%s] Skipping because unable to annotate node: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
continue
}
} else {
Expand All @@ -121,8 +118,7 @@ func HandleRollingUpgrade(kubernetesClient k8s.KubernetesClientApi, ec2Service e
log.Printf("[%s][%s] Draining node", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId))
err := kubernetesClient.Drain(node.Name, config.Get().IgnoreDaemonSets, config.Get().DeleteLocalData)
if err != nil {
log.Printf("[%s][%s] Ran into error while draining node: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
log.Printf("[%s][%s] Skipping", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId))
log.Printf("[%s][%s] Skipping because ran into error while draining node: %v", aws.StringValue(autoScalingGroup.AutoScalingGroupName), aws.StringValue(outdatedInstance.InstanceId), err.Error())
continue
} else {
// Only annotate if no error was encountered
Expand Down
Loading

0 comments on commit 2c85128

Please sign in to comment.