Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add ignoreMinNodesCheck to allow disabling minimum node size check #1350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add ignoreMinNodesCheck to allow disabling minimum node size check #1350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ These are top level keys in the Descheduler Policy that you can use to configure
| `nodeSelector` |`string`| `nil` | limiting the nodes which are processed. Only used when `nodeFit`=`true` and only by the PreEvictionFilter Extension Point |
| `maxNoOfPodsToEvictPerNode` |`int`| `nil` | maximum number of pods evicted from each node (summed through all strategies) |
| `maxNoOfPodsToEvictPerNamespace` |`int`| `nil` | maximum number of pods evicted from each namespace (summed through all strategies) |
| `ignoreMinNodesCheck` |`bool`| `nil` | descheduler requires more than one node in the cluster, setting this to `true` will disable the check |

### Evictor Plugin configuration (Default Evictor)

Expand Down
1 change: 1 addition & 0 deletions docs/deprecated/v1alpha1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ evictLocalStoragePods: true
evictSystemCriticalPods: true
maxNoOfPodsToEvictPerNode: 40
ignorePvcPods: false
ignoreMinNodesCheck: false
strategies:
...
```
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type DeschedulerPolicy struct {

// MaxNoOfPodsToEvictPerNamespace restricts maximum of pods to be evicted per namespace.
MaxNoOfPodsToEvictPerNamespace *uint

// IgnoreMinNodesCheck allows skipping the min nodes check, if set to true.
IgnoreMinNodesCheck *bool
}

// Namespaces carries a list of included/excluded namespaces
Expand Down
1 change: 1 addition & 0 deletions pkg/api/v1alpha1/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ func V1alpha1ToInternal(
out.NodeSelector = deschedulerPolicy.NodeSelector
out.MaxNoOfPodsToEvictPerNamespace = deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace
out.MaxNoOfPodsToEvictPerNode = deschedulerPolicy.MaxNoOfPodsToEvictPerNode
out.IgnoreMinNodesCheck = deschedulerPolicy.IgnoreMinNodesCheck

return nil
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type DeschedulerPolicy struct {

// MaxNoOfPodsToEvictPerNamespace restricts maximum of pods to be evicted per namespace.
MaxNoOfPodsToEvictPerNamespace *uint `json:"maxNoOfPodsToEvictPerNamespace,omitempty"`

// IgnoreMinNodesCheck allows skipping the min nodes check, if set to true.
IgnoreMinNodesCheck *bool `json:"ignoreMinNodesCheck,omitempty"`
}

type (
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/api/v1alpha2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ type DeschedulerPolicy struct {

// MaxNoOfPodsToEvictPerNamespace restricts maximum of pods to be evicted per namespace.
MaxNoOfPodsToEvictPerNamespace *uint `json:"maxNoOfPodsToEvictPerNamespace,omitempty"`

// IgnoreMinNodesCheck allows skipping the min nodes check, if set to true.
IgnoreMinNodesCheck *bool `json:"ignoreMinNodesCheck,omitempty"`
}

type DeschedulerProfile struct {
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1alpha2/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/api/v1alpha2/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/api/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 9 additions & 8 deletions pkg/descheduler/descheduler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,25 @@ import (

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/client-go/discovery"
"k8s.io/client-go/informers"
"k8s.io/client-go/tools/events"
componentbaseconfig "k8s.io/component-base/config"
"k8s.io/klog/v2"

v1 "k8s.io/api/core/v1"
policy "k8s.io/api/policy/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
utilversion "k8s.io/apimachinery/pkg/util/version"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/discovery"
"k8s.io/client-go/informers"
clientset "k8s.io/client-go/kubernetes"
fakeclientset "k8s.io/client-go/kubernetes/fake"
listersv1 "k8s.io/client-go/listers/core/v1"
schedulingv1 "k8s.io/client-go/listers/scheduling/v1"
core "k8s.io/client-go/testing"
"k8s.io/client-go/tools/events"
componentbaseconfig "k8s.io/component-base/config"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

"sigs.k8s.io/descheduler/pkg/descheduler/client"
eutils "sigs.k8s.io/descheduler/pkg/descheduler/evictions/utils"
Expand Down Expand Up @@ -116,8 +116,9 @@ func (d *descheduler) runDeschedulerLoop(ctx context.Context, nodes []*v1.Node)
metrics.DeschedulerLoopDuration.With(map[string]string{}).Observe(time.Since(loopStartDuration).Seconds())
}(time.Now())

shouldCheckMinNodes := !ptr.Deref(d.deschedulerPolicy.IgnoreMinNodesCheck, false)
// if len is still <= 1 error out
if len(nodes) <= 1 {
if shouldCheckMinNodes && len(nodes) <= 1 {
Copy link
Contributor

@ingvagabund ingvagabund Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we could limit this check only for plugins with Balance extension point. Normally, any Deschedule EP should be allowed to operate over any node independent of how many there are.

From #1347:

It is useful to me to automatically delete pods with too many restart or failed pods.

This would still fit the case. This will also cover the case in #1298. The descheduler will keep spinning the descheduling cycle and skip execution of any Balance EP if len(nodes) < 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good feedback. That makes sense to me

I can take a stab at this soon

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx. Lemme know once you need a review. I'll take a look.

klog.V(1).InfoS("The cluster size is 0 or 1 meaning eviction causes service disruption or degradation. So aborting..")
return fmt.Errorf("the cluster size is 0 or 1")
}
Expand Down
61 changes: 61 additions & 0 deletions pkg/descheduler/descheduler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ import (
"k8s.io/apimachinery/pkg/runtime"
apiversion "k8s.io/apimachinery/pkg/version"
fakediscovery "k8s.io/client-go/discovery/fake"
"k8s.io/client-go/informers"
fakeclientset "k8s.io/client-go/kubernetes/fake"
core "k8s.io/client-go/testing"
"k8s.io/client-go/tools/events"
"k8s.io/utils/ptr"
"sigs.k8s.io/descheduler/cmd/descheduler/app/options"
"sigs.k8s.io/descheduler/pkg/api"
"sigs.k8s.io/descheduler/pkg/api/v1alpha1"
Expand Down Expand Up @@ -248,6 +251,64 @@ func TestRootCancelWithNoInterval(t *testing.T) {
}
}

func TestRunDeschedulerLoop(t *testing.T) {
n1 := test.BuildTestNode("n1", 2000, 3000, 10, nil)
n2 := test.BuildTestNode("n2", 2000, 3000, 10, nil)

testCases := map[string]struct {
expectError bool
deschedulerPolicy *api.DeschedulerPolicy
nodes []*v1.Node
}{
"nodes=1, IgnoreMinNodesCheck=true, no error": {
expectError: false,
deschedulerPolicy: &api.DeschedulerPolicy{IgnoreMinNodesCheck: ptr.To[bool](true)},
nodes: []*v1.Node{n1},
},
"nodes=1, IgnoreMinNodesCheck=false, has error": {
expectError: true,
deschedulerPolicy: &api.DeschedulerPolicy{IgnoreMinNodesCheck: ptr.To[bool](false)},
nodes: []*v1.Node{n1},
},
"nodes=1, IgnoreMinNodesCheck=nil, has error": {
expectError: true,
deschedulerPolicy: &api.DeschedulerPolicy{IgnoreMinNodesCheck: nil},
nodes: []*v1.Node{n1},
},
"nodes=2, IgnoreMinNodesCheck=false, no error": {
expectError: false,
deschedulerPolicy: &api.DeschedulerPolicy{IgnoreMinNodesCheck: ptr.To[bool](false)},
nodes: []*v1.Node{n1, n2},
},
"nodes=2, IgnoreMinNodesCheck=true, no error": {
expectError: false,
deschedulerPolicy: &api.DeschedulerPolicy{IgnoreMinNodesCheck: ptr.To[bool](true)},
nodes: []*v1.Node{n1, n2},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
ctx := context.Background()
fakeClient := fakeclientset.NewSimpleClientset(n1, n2)

eventRecorder := &events.FakeRecorder{}
sharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0)

instance, err := newDescheduler(ctx, &options.DeschedulerServer{}, tc.deschedulerPolicy, "v1", eventRecorder, sharedInformerFactory)
if err != nil {
t.Fatalf("unable to initialize descheduler")
}

err = instance.runDeschedulerLoop(ctx, tc.nodes)
hasError := err != nil
if tc.expectError != hasError {
t.Errorf("unexpected descheduler loop behavior. err %v", err)
}
})
}
}

func TestValidateVersionCompatibility(t *testing.T) {
type testCase struct {
name string
Expand Down
Loading