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.

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

add v1alpha2 registry based conversion #1006

Merged
5 changes: 4 additions & 1 deletion pkg/api/conversion.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/*
Copyright 2017 The Kubernetes Authors.
Copyright 2022 The Kubernetes Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

hate to do this, but these need to be updated to 2023 now in each new file


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.
Expand Down
227 changes: 227 additions & 0 deletions pkg/api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,230 @@ limitations under the License.
*/

package v1alpha1

import (
"context"
"fmt"

v1 "k8s.io/api/core/v1"
"k8s.io/client-go/informers"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
"sigs.k8s.io/descheduler/pkg/api"
"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
"sigs.k8s.io/descheduler/pkg/framework"
"sigs.k8s.io/descheduler/pkg/framework/pluginregistry"
"sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor"
"sigs.k8s.io/descheduler/pkg/utils"
)

// evictorImpl implements the Evictor interface so plugins
// can evict a pod without importing a specific pod evictor
type evictorImpl struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do evictorImpl and other types need to be present under pkg/api/v1alpha1 if they are expected to be used solely by the framework?

Copy link
Contributor Author

@knelasevero knelasevero Jan 5, 2023

Choose a reason for hiding this comment

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

I moved things here by Mike's request: #1006 (comment)

/*197*/ pluginInstance, err := registry[string(name)].PluginBuilder(pluginArgs, &handleImpl{})

These lines need it. And I can't import it from somewhere else since basically all other packages import v1alpha1 and this would generate a cycle. When this was back at policyconfig.go we could just use it.

podEvictor *evictions.PodEvictor
evictorFilter framework.EvictorPlugin
}

var _ framework.Evictor = &evictorImpl{}

// Filter checks if a pod can be evicted
func (ei *evictorImpl) Filter(pod *v1.Pod) bool {
return ei.evictorFilter.Filter(pod)
}

// PreEvictionFilter checks if pod can be evicted right before eviction
func (ei *evictorImpl) PreEvictionFilter(pod *v1.Pod) bool {
return ei.evictorFilter.PreEvictionFilter(pod)
}

// Evict evicts a pod (no pre-check performed)
func (ei *evictorImpl) Evict(ctx context.Context, pod *v1.Pod, opts evictions.EvictOptions) bool {
return ei.podEvictor.EvictPod(ctx, pod, opts)
}

func (ei *evictorImpl) NodeLimitExceeded(node *v1.Node) bool {
return ei.podEvictor.NodeLimitExceeded(node)
}

// handleImpl implements the framework handle which gets passed to plugins
type handleImpl struct {
clientSet clientset.Interface
getPodsAssignedToNodeFunc podutil.GetPodsAssignedToNodeFunc
sharedInformerFactory informers.SharedInformerFactory
evictor *evictorImpl
}

var _ framework.Handle = &handleImpl{}

// ClientSet retrieves kube client set
func (hi *handleImpl) ClientSet() clientset.Interface {
return hi.clientSet
}

// GetPodsAssignedToNodeFunc retrieves GetPodsAssignedToNodeFunc implementation
func (hi *handleImpl) GetPodsAssignedToNodeFunc() podutil.GetPodsAssignedToNodeFunc {
return hi.getPodsAssignedToNodeFunc
}

// SharedInformerFactory retrieves shared informer factory
func (hi *handleImpl) SharedInformerFactory() informers.SharedInformerFactory {
return hi.sharedInformerFactory
}

// Evictor retrieves evictor so plugins can filter and evict pods
func (hi *handleImpl) Evictor() framework.Evictor {
return hi.evictor
}

func V1alpha1ToInternal(
client clientset.Interface,
deschedulerPolicy *DeschedulerPolicy,
registry pluginregistry.Registry,
) (*api.DeschedulerPolicy, error) {
var evictLocalStoragePods bool
if deschedulerPolicy.EvictLocalStoragePods != nil {
evictLocalStoragePods = *deschedulerPolicy.EvictLocalStoragePods
}

evictBarePods := false
if deschedulerPolicy.EvictFailedBarePods != nil {
evictBarePods = *deschedulerPolicy.EvictFailedBarePods
if evictBarePods {
klog.V(1).InfoS("Warning: EvictFailedBarePods is set to True. This could cause eviction of pods without ownerReferences.")
Copy link
Contributor

Choose a reason for hiding this comment

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

no keysAndValues args so not structured logs.

Suggested change
klog.V(1).InfoS("Warning: EvictFailedBarePods is set to True. This could cause eviction of pods without ownerReferences.")
klog.V(1).Info("Warning: EvictFailedBarePods is set to True. This could cause eviction of pods without ownerReferences.")

}
}

evictSystemCriticalPods := false
if deschedulerPolicy.EvictSystemCriticalPods != nil {
evictSystemCriticalPods = *deschedulerPolicy.EvictSystemCriticalPods
if evictSystemCriticalPods {
klog.V(1).InfoS("Warning: EvictSystemCriticalPods is set to True. This could cause eviction of Kubernetes system pods.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
klog.V(1).InfoS("Warning: EvictSystemCriticalPods is set to True. This could cause eviction of Kubernetes system pods.")
klog.V(1).Info("Warning: EvictSystemCriticalPods is set to True. This could cause eviction of Kubernetes system pods.")

}
}

ignorePvcPods := false
if deschedulerPolicy.IgnorePVCPods != nil {
ignorePvcPods = *deschedulerPolicy.IgnorePVCPods
}

var profiles []api.Profile

// Build profiles
for name, strategy := range deschedulerPolicy.Strategies {
if _, ok := pluginregistry.PluginRegistry[string(name)]; ok {
if strategy.Enabled {
params := strategy.Params
if params == nil {
params = &StrategyParameters{}
}

nodeFit := false
if name != "PodLifeTime" {
nodeFit = params.NodeFit
}

if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
klog.ErrorS(fmt.Errorf("priority threshold misconfigured"), "only one of priorityThreshold fields can be set", "pluginName", name)
return nil, fmt.Errorf("priority threshold misconfigured for plugin %v", name)
}

var priorityThreshold *api.PriorityThreshold
if strategy.Params != nil {
priorityThreshold = &api.PriorityThreshold{
Value: strategy.Params.ThresholdPriority,
Name: strategy.Params.ThresholdPriorityClassName,
}
}
thresholdPriority, err := utils.GetPriorityFromStrategyParams(context.TODO(), client, priorityThreshold)
if err != nil {
klog.ErrorS(err, "Failed to get threshold priority from strategy's params")
return nil, fmt.Errorf("failed to get threshold priority from strategy's params: %v", err)
}

var pluginConfig *api.PluginConfig
if pcFnc, exists := StrategyParamsToPluginArgs[string(name)]; exists {
pluginConfig, err = pcFnc(params)
if err != nil {
klog.ErrorS(err, "skipping strategy", "strategy", name)
return nil, fmt.Errorf("failed to get plugin config for strategy %v: %v", name, err)
}
} else {
klog.ErrorS(fmt.Errorf("unknown strategy name"), "skipping strategy", "strategy", name)
return nil, fmt.Errorf("unknown strategy name: %v", name)
}

profile := api.Profile{
Name: fmt.Sprintf("strategy-%v-profile", name),
PluginConfigs: []api.PluginConfig{
{
Name: defaultevictor.PluginName,
Args: &defaultevictor.DefaultEvictorArgs{
EvictLocalStoragePods: evictLocalStoragePods,
EvictSystemCriticalPods: evictSystemCriticalPods,
IgnorePvcPods: ignorePvcPods,
EvictFailedBarePods: evictBarePods,
NodeFit: nodeFit,
PriorityThreshold: &api.PriorityThreshold{
Value: &thresholdPriority,
},
},
},
*pluginConfig,
},
Plugins: api.Plugins{
Evict: api.PluginSet{
Enabled: []string{defaultevictor.PluginName},
},
},
}

pluginArgs := registry[string(name)].PluginArgInstance
pluginInstance, err := registry[string(name)].PluginBuilder(pluginArgs, &handleImpl{})
if err != nil {
klog.ErrorS(fmt.Errorf("could not build plugin"), "plugin build error", "plugin", name)
return nil, fmt.Errorf("could not build plugin: %v", name)
}

// pluginInstance can be of any of each type, or both
profilePlugins := profile.Plugins
profile.Plugins = enableProfilePluginsByType(profilePlugins, pluginInstance, pluginConfig)
profiles = append(profiles, profile)
}
} else {
klog.ErrorS(fmt.Errorf("unknown strategy name"), "skipping strategy", "strategy", name)
return nil, fmt.Errorf("unknown strategy name: %v", name)
}
}

return &api.DeschedulerPolicy{
Profiles: profiles,
NodeSelector: deschedulerPolicy.NodeSelector,
MaxNoOfPodsToEvictPerNode: deschedulerPolicy.MaxNoOfPodsToEvictPerNode,
MaxNoOfPodsToEvictPerNamespace: deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace,
}, nil
}

func enableProfilePluginsByType(profilePlugins api.Plugins, pluginInstance framework.Plugin, pluginConfig *api.PluginConfig) api.Plugins {
profilePlugins = checkBalance(profilePlugins, pluginInstance, pluginConfig)
profilePlugins = checkDeschedule(profilePlugins, pluginInstance, pluginConfig)
return profilePlugins
}

func checkBalance(profilePlugins api.Plugins, pluginInstance framework.Plugin, pluginConfig *api.PluginConfig) api.Plugins {
_, ok := pluginInstance.(framework.BalancePlugin)
if ok {
klog.V(3).Info("converting Balance plugin: %s", pluginInstance.Name())
profilePlugins.Balance.Enabled = []string{pluginConfig.Name}
}
return profilePlugins
}

func checkDeschedule(profilePlugins api.Plugins, pluginInstance framework.Plugin, pluginConfig *api.PluginConfig) api.Plugins {
_, ok := pluginInstance.(framework.DeschedulePlugin)
if ok {
klog.V(3).Info("converting Deschedule plugin: %s", pluginInstance.Name())
profilePlugins.Deschedule.Enabled = []string{pluginConfig.Name}
}
return profilePlugins
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ See the License for the specific language governing permissions and
limitations under the License.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why renaming the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember now 😅 I think because all other files have two words together and none of the other files have underscores.

*/

package descheduler
// This is a temporary package that will only exist while we have both v1alpha1 and v1alpha2
package v1alpha1

import (
"fmt"

"k8s.io/klog/v2"
"sigs.k8s.io/descheduler/pkg/api"
"sigs.k8s.io/descheduler/pkg/api/v1alpha1"
"sigs.k8s.io/descheduler/pkg/framework/plugins/nodeutilization"
"sigs.k8s.io/descheduler/pkg/framework/plugins/podlifetime"
"sigs.k8s.io/descheduler/pkg/framework/plugins/removeduplicates"
Expand All @@ -37,8 +37,8 @@ import (
// without any wiring. Keeping the wiring here so the descheduler can still use
// the v1alpha1 configuration during the strategy migration to plugins.

var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error){
"RemovePodsViolatingNodeTaints": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
var StrategyParamsToPluginArgs = map[string]func(params *StrategyParameters) (*api.PluginConfig, error){
"RemovePodsViolatingNodeTaints": func(params *StrategyParameters) (*api.PluginConfig, error) {
args := &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
LabelSelector: params.LabelSelector,
Expand All @@ -54,10 +54,10 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemoveFailedPods": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemoveFailedPods": func(params *StrategyParameters) (*api.PluginConfig, error) {
failedPodsParams := params.FailedPods
if failedPodsParams == nil {
failedPodsParams = &v1alpha1.FailedPods{}
failedPodsParams = &FailedPods{}
}
args := &removefailedpods.RemoveFailedPodsArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
Expand All @@ -76,7 +76,7 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemovePodsViolatingNodeAffinity": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemovePodsViolatingNodeAffinity": func(params *StrategyParameters) (*api.PluginConfig, error) {
args := &removepodsviolatingnodeaffinity.RemovePodsViolatingNodeAffinityArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
LabelSelector: params.LabelSelector,
Expand All @@ -91,7 +91,7 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemovePodsViolatingInterPodAntiAffinity": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemovePodsViolatingInterPodAntiAffinity": func(params *StrategyParameters) (*api.PluginConfig, error) {
args := &removepodsviolatinginterpodantiaffinity.RemovePodsViolatingInterPodAntiAffinityArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
LabelSelector: params.LabelSelector,
Expand All @@ -105,10 +105,10 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemovePodsHavingTooManyRestarts": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemovePodsHavingTooManyRestarts": func(params *StrategyParameters) (*api.PluginConfig, error) {
tooManyRestartsParams := params.PodsHavingTooManyRestarts
if tooManyRestartsParams == nil {
tooManyRestartsParams = &v1alpha1.PodsHavingTooManyRestarts{}
tooManyRestartsParams = &PodsHavingTooManyRestarts{}
}
args := &removepodshavingtoomanyrestarts.RemovePodsHavingTooManyRestartsArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
Expand All @@ -125,10 +125,10 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"PodLifeTime": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"PodLifeTime": func(params *StrategyParameters) (*api.PluginConfig, error) {
podLifeTimeParams := params.PodLifeTime
if podLifeTimeParams == nil {
podLifeTimeParams = &v1alpha1.PodLifeTime{}
podLifeTimeParams = &PodLifeTime{}
}

var states []string
Expand All @@ -154,7 +154,7 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemoveDuplicates": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemoveDuplicates": func(params *StrategyParameters) (*api.PluginConfig, error) {
args := &removeduplicates.RemoveDuplicatesArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
}
Expand All @@ -170,7 +170,7 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"RemovePodsViolatingTopologySpreadConstraint": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"RemovePodsViolatingTopologySpreadConstraint": func(params *StrategyParameters) (*api.PluginConfig, error) {
args := &removepodsviolatingtopologyspreadconstraint.RemovePodsViolatingTopologySpreadConstraintArgs{
Namespaces: v1alpha1NamespacesToInternal(params.Namespaces),
LabelSelector: params.LabelSelector,
Expand All @@ -185,9 +185,9 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"HighNodeUtilization": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"HighNodeUtilization": func(params *StrategyParameters) (*api.PluginConfig, error) {
if params.NodeResourceUtilizationThresholds == nil {
params.NodeResourceUtilizationThresholds = &v1alpha1.NodeResourceUtilizationThresholds{}
params.NodeResourceUtilizationThresholds = &NodeResourceUtilizationThresholds{}
}
args := &nodeutilization.HighNodeUtilizationArgs{
EvictableNamespaces: v1alpha1NamespacesToInternal(params.Namespaces),
Expand All @@ -203,9 +203,9 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
Args: args,
}, nil
},
"LowNodeUtilization": func(params *v1alpha1.StrategyParameters) (*api.PluginConfig, error) {
"LowNodeUtilization": func(params *StrategyParameters) (*api.PluginConfig, error) {
if params.NodeResourceUtilizationThresholds == nil {
params.NodeResourceUtilizationThresholds = &v1alpha1.NodeResourceUtilizationThresholds{}
params.NodeResourceUtilizationThresholds = &NodeResourceUtilizationThresholds{}
}
args := &nodeutilization.LowNodeUtilizationArgs{
EvictableNamespaces: v1alpha1NamespacesToInternal(params.Namespaces),
Expand All @@ -226,7 +226,7 @@ var strategyParamsToPluginArgs = map[string]func(params *v1alpha1.StrategyParame
},
}

func v1alpha1NamespacesToInternal(namespaces *v1alpha1.Namespaces) *api.Namespaces {
func v1alpha1NamespacesToInternal(namespaces *Namespaces) *api.Namespaces {
internal := &api.Namespaces{}
if namespaces != nil {
if namespaces.Exclude != nil {
Expand All @@ -241,7 +241,7 @@ func v1alpha1NamespacesToInternal(namespaces *v1alpha1.Namespaces) *api.Namespac
return internal
}

func v1alpha1ThresholdToInternal(thresholds v1alpha1.ResourceThresholds) api.ResourceThresholds {
func v1alpha1ThresholdToInternal(thresholds ResourceThresholds) api.ResourceThresholds {
internal := make(api.ResourceThresholds, len(thresholds))
for k, v := range thresholds {
internal[k] = api.Percentage(float64(v))
Expand Down
Loading