From b7672fc0df4f50c10ad026a39ff86e8c8765b074 Mon Sep 17 00:00:00 2001 From: Yaroslava Serdiuk Date: Tue, 2 Jan 2024 11:04:23 +0000 Subject: [PATCH] Review --- .../podlistprocessor/pod_list_processor.go | 8 ++--- cluster-autoscaler/main.go | 2 +- .../processors/pods/pod_list_processor.go | 29 +++++++++++-------- .../provisioning_request_processors.go | 2 +- .../provreqclient/client.go | 9 +----- 5 files changed, 23 insertions(+), 27 deletions(-) diff --git a/cluster-autoscaler/core/podlistprocessor/pod_list_processor.go b/cluster-autoscaler/core/podlistprocessor/pod_list_processor.go index f14860983475..78bd83f99d4b 100644 --- a/cluster-autoscaler/core/podlistprocessor/pod_list_processor.go +++ b/cluster-autoscaler/core/podlistprocessor/pod_list_processor.go @@ -23,14 +23,12 @@ import ( // NewDefaultPodListProcessor returns a default implementation of the pod list // processor, which wraps and sequentially runs other sub-processors. -func NewDefaultPodListProcessor(predicateChecker predicatechecker.PredicateChecker) *pods.ListedPodListProcessor { - return &pods.ListedPodListProcessor{ - Processors: []pods.PodListProcessor{ +func NewDefaultPodListProcessor(predicateChecker predicatechecker.PredicateChecker) *pods.CombinedPodListProcessor { + return pods.NewCombinedPodListProcessor([]pods.PodListProcessor{ NewClearTPURequestsPodListProcessor(), NewFilterOutExpendablePodListProcessor(), NewCurrentlyDrainedNodesPodListProcessor(), NewFilterOutSchedulablePodListProcessor(predicateChecker), NewFilterOutDaemonSetPodListProcessor(), - }, - } + }) } diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index 5c1a07e6327e..99fa87810540 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -480,7 +480,7 @@ func buildAutoscaler(debuggingSnapshotter debuggingsnapshot.DebuggingSnapshotter opts.Processors.TemplateNodeInfoProvider = nodeinfosprovider.NewDefaultTemplateNodeInfoProvider(nodeInfoCacheExpireTime, *forceDaemonSets) podListProcessor := podlistprocessor.NewDefaultPodListProcessor(opts.PredicateChecker) if autoscalingOptions.ProvisioningRequestEnabled { - podListProcessor.AppendProcessor(provreq.NewProvisioningRequestPodsFilter(provreq.NewDefautlEventManager())) + podListProcessor.AddProcessor(provreq.NewProvisioningRequestPodsFilter(provreq.NewDefautlEventManager())) } opts.Processors.PodListProcessor = podListProcessor scaleDownCandidatesComparers := []scaledowncandidates.CandidatesComparer{} diff --git a/cluster-autoscaler/processors/pods/pod_list_processor.go b/cluster-autoscaler/processors/pods/pod_list_processor.go index cd3a5a3fd45e..37a7426125c5 100644 --- a/cluster-autoscaler/processors/pods/pod_list_processor.go +++ b/cluster-autoscaler/processors/pods/pod_list_processor.go @@ -49,15 +49,25 @@ func (p *NoOpPodListProcessor) Process( func (p *NoOpPodListProcessor) CleanUp() { } -// ListedPodListProcessor is a list of PodListProcessors -type ListedPodListProcessor struct { - Processors []PodListProcessor +// CombinedPodListProcessor is a list of PodListProcessors +type CombinedPodListProcessor struct { + processors []PodListProcessor +} + +//NewCombinedPodListProcessor construct CombinedPodListProcessor. +func NewCombinedPodListProcessor(processors []PodListProcessor) *CombinedPodListProcessor { + return &CombinedPodListProcessor{processors} +} + +// AddProcessor append processor to the list. +func (p *CombinedPodListProcessor) AddProcessor(processor PodListProcessor) { + p.processors = append(p.processors, processor) } // Process runs sub-processors sequentially -func (p *ListedPodListProcessor) Process(ctx *context.AutoscalingContext, unschedulablePods []*apiv1.Pod) ([]*apiv1.Pod, error) { +func (p *CombinedPodListProcessor) Process(ctx *context.AutoscalingContext, unschedulablePods []*apiv1.Pod) ([]*apiv1.Pod, error) { var err error - for _, processor := range p.Processors { + for _, processor := range p.processors { unschedulablePods, err = processor.Process(ctx, unschedulablePods) if err != nil { return nil, err @@ -67,13 +77,8 @@ func (p *ListedPodListProcessor) Process(ctx *context.AutoscalingContext, unsche } // CleanUp cleans up the processor's internal structures. -func (p *ListedPodListProcessor) CleanUp() { - for _, processor := range p.Processors { +func (p *CombinedPodListProcessor) CleanUp() { + for _, processor := range p.processors { processor.CleanUp() } } - -// AppendProcessor append processor to the list. -func (p *ListedPodListProcessor) AppendProcessor(processor PodListProcessor) { - p.Processors = append(p.Processors, processor) -} diff --git a/cluster-autoscaler/processors/provreq/provisioning_request_processors.go b/cluster-autoscaler/processors/provreq/provisioning_request_processors.go index d7bee0fa63e1..c48d489c0746 100644 --- a/cluster-autoscaler/processors/provreq/provisioning_request_processors.go +++ b/cluster-autoscaler/processors/provreq/provisioning_request_processors.go @@ -50,7 +50,7 @@ func NewDefautlEventManager() *defaultEventManager { // LogIgnoredInScaleUpEvent adds event about ignored scale up for unscheduled pod, that consumes Provisioning Request. func (e *defaultEventManager) LogIgnoredInScaleUpEvent(context *context.AutoscalingContext, now time.Time, pod *apiv1.Pod, prName string) { - message := fmt.Sprintf("Unschedulable pod ignored in scale-up loop, because it's consuming ProvisioningRequest %s/%s", pod.Namespace, prName) + message := fmt.Sprintf("Unschedulable pod didn't trigger scale-up, because it's consuming ProvisioningRequest %s/%s", pod.Namespace, prName) if e.loggedEvents < e.limit { context.Recorder.Event(pod, apiv1.EventTypeNormal, "", message) e.loggedEvents++ diff --git a/cluster-autoscaler/provisioningrequest/provreqclient/client.go b/cluster-autoscaler/provisioningrequest/provreqclient/client.go index db1852b35c51..f11c5cd42555 100644 --- a/cluster-autoscaler/provisioningrequest/provreqclient/client.go +++ b/cluster-autoscaler/provisioningrequest/provreqclient/client.go @@ -41,13 +41,6 @@ const ( provisioningRequestClientCallTimeout = 4 * time.Second ) -// ProvisioningRequestClient represents the service that is able to list -// and access different Provisioning Requests. -type ProvisioningRequestClient interface { - ProvisioningRequest(namespace, name string) (*provreqwrapper.ProvisioningRequest, error) - ProvisioningRequests() ([]*provreqwrapper.ProvisioningRequest, error) -} - // ProvisioningRequestClientV1beta1 represents client for v1beta1 ProvReq CRD. type ProvisioningRequestClientV1beta1 struct { client versioned.Interface @@ -56,7 +49,7 @@ type ProvisioningRequestClientV1beta1 struct { } // NewProvisioningRequestClient configures and returns a provisioningRequestClient. -func NewProvisioningRequestClient(kubeConfig *rest.Config) (ProvisioningRequestClient, error) { +func NewProvisioningRequestClient(kubeConfig *rest.Config) (*ProvisioningRequestClientV1beta1, error) { prClient, err := newPRClient(kubeConfig) if err != nil { return nil, fmt.Errorf("Failed to create Provisioning Request client: %v", err)