Skip to content

Commit

Permalink
Convert scale-down pdb check to drainability rule
Browse files Browse the repository at this point in the history
  • Loading branch information
artemvmin committed Sep 30, 2023
1 parent 5005c3b commit 1775757
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 17 deletions.
26 changes: 9 additions & 17 deletions cluster-autoscaler/simulator/drain.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package simulator

import (
"fmt"
"time"

apiv1 "k8s.io/api/core/v1"
Expand All @@ -31,14 +30,14 @@ import (
schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
)

// GetPodsToMove returns a list of pods that should be moved elsewhere
// and a list of DaemonSet pods that should be evicted if the node
// is drained. Raises error if there is an unreplicated pod.
// Based on kubectl drain code. If listers is nil it makes an assumption that RC, DS, Jobs and RS were deleted
// along with their pods (no abandoned pods with dangling created-by annotation).
// If listers is not nil it checks whether RC, DS, Jobs and RS that created these pods
// still exist.
// TODO(x13n): Rewrite GetPodsForDeletionOnNodeDrain into a set of DrainabilityRules.
// GetPodsToMove returns a list of pods that should be moved elsewhere and a
// list of DaemonSet pods that should be evicted if the node is drained.
// Raises error if there is an unreplicated pod.
// Based on kubectl drain code. If listers is nil it makes an assumption that
// RC, DS, Jobs and RS were deleted along with their pods (no abandoned pods
// with dangling created-by annotation).
// If listers is not nil it checks whether RC, DS, Jobs and RS that created
// these pods still exist.
func GetPodsToMove(nodeInfo *schedulerframework.NodeInfo, deleteOptions options.NodeDeleteOptions, drainabilityRules rules.Rules, listers kube_util.ListerRegistry, remainingPdbTracker pdb.RemainingPdbTracker, timestamp time.Time) (pods []*apiv1.Pod, daemonSetPods []*apiv1.Pod, blockingPod *drain.BlockingPod, err error) {
var drainPods, drainDs []*apiv1.Pod
if drainabilityRules == nil {
Expand Down Expand Up @@ -82,13 +81,6 @@ func GetPodsToMove(nodeInfo *schedulerframework.NodeInfo, deleteOptions options.
deleteOptions.SkipNodesWithLocalStorage,
deleteOptions.SkipNodesWithCustomControllerPods,
timestamp)
pods = append(pods, drainPods...)
daemonSetPods = append(daemonSetPods, drainDs...)

if canRemove, _, blockingPodInfo := remainingPdbTracker.CanRemovePods(pods); !canRemove {
pod := blockingPodInfo.Pod
return []*apiv1.Pod{}, []*apiv1.Pod{}, blockingPodInfo, fmt.Errorf("not enough pod disruption budget to move %s/%s", pod.Namespace, pod.Name)
}

return pods, daemonSetPods, nil, nil
return append(pods, drainPods...), append(daemonSetPods, drainDs...), nil, nil
}
43 changes: 43 additions & 0 deletions cluster-autoscaler/simulator/drainability/rules/pdb/rule.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package pdb

import (
"fmt"

apiv1 "k8s.io/api/core/v1"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability"
"k8s.io/autoscaler/cluster-autoscaler/utils/drain"
)

// Rule is a drainability rule on how to handle pods with pdbs.
type Rule struct{}

// New creates a new Rule.
func New() *Rule {
return &Rule{}
}

// Drainable decides how to handle pods with pdbs on node drain.
func (Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod) drainability.Status {
for _, pdb := range drainCtx.RemainingPdbTracker.MatchingPdbs(pod) {
if pdb.Status.DisruptionsAllowed < 1 {
return drainability.NewBlockedStatus(drain.NotEnoughPdb, fmt.Errorf("not enough pod disruption budget to move %s/%s", pod.Namespace, pod.Name))
}
}
return drainability.NewUndefinedStatus()
}
155 changes: 155 additions & 0 deletions cluster-autoscaler/simulator/drainability/rules/pdb/rule_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package pdb

import (
"testing"

apiv1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/autoscaler/cluster-autoscaler/core/scaledown/pdb"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability"
"k8s.io/autoscaler/cluster-autoscaler/utils/drain"
)

func TestRule(t *testing.T) {
one := intstr.FromInt(1)

testCases := []struct {
desc string
pod *apiv1.Pod
pdbs []*policyv1.PodDisruptionBudget
wantOutcome drainability.OutcomeType
wantReason drain.BlockingPodReason
}{
{
desc: "no pdbs",
pod: &apiv1.Pod{},
},
{
desc: "no matching pdbs",
pod: &apiv1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "happy",
Namespace: "good",
Labels: map[string]string{
"label": "true",
},
},
},
pdbs: []*policyv1.PodDisruptionBudget{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "bad",
},
Spec: policyv1.PodDisruptionBudgetSpec{
MinAvailable: &one,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "true",
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "good",
},
Spec: policyv1.PodDisruptionBudgetSpec{
MinAvailable: &one,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "false",
},
},
},
},
},
},
{
desc: "pdb prevents scale-down",
pod: &apiv1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "sad",
Namespace: "good",
Labels: map[string]string{
"label": "true",
},
},
},
pdbs: []*policyv1.PodDisruptionBudget{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "bad",
},
Spec: policyv1.PodDisruptionBudgetSpec{
MinAvailable: &one,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "true",
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "good",
},
Spec: policyv1.PodDisruptionBudgetSpec{
MinAvailable: &one,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "true",
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "good",
},
Spec: policyv1.PodDisruptionBudgetSpec{
MinAvailable: &one,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "false",
},
},
},
},
},
wantOutcome: drainability.BlockDrain,
wantReason: drain.NotEnoughPdb,
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
tracker := pdb.NewBasicRemainingPdbTracker()
tracker.SetPdbs(tc.pdbs)
drainCtx := &drainability.DrainContext{
RemainingPdbTracker: tracker,
}

got := New().Drainable(drainCtx, tc.pod)
if got.Outcome != tc.wantOutcome || got.BlockingReason != tc.wantReason {
t.Errorf("Rule.Drainable(%s) = (outcome: %v, reason: %v), want (outcome: %v, reason: %v)", tc.pod.Name, got.Outcome, got.BlockingReason, tc.wantOutcome, tc.wantReason)
}
})
}
}
2 changes: 2 additions & 0 deletions cluster-autoscaler/simulator/drainability/rules/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/localstorage"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/mirror"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/notsafetoevict"
pdbrule "k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/pdb"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/replicated"
"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/system"
)
Expand All @@ -44,6 +45,7 @@ func Default() Rules {
system.New(),
notsafetoevict.New(),
localstorage.New(),
pdbrule.New(),
}
}

Expand Down

0 comments on commit 1775757

Please sign in to comment.