diff --git a/cluster-autoscaler/provisioningrequest/besteffortatomic/provisioning_class.go b/cluster-autoscaler/provisioningrequest/besteffortatomic/provisioning_class.go index 0c7091c92e56..4adf8471d427 100644 --- a/cluster-autoscaler/provisioningrequest/besteffortatomic/provisioning_class.go +++ b/cluster-autoscaler/provisioningrequest/besteffortatomic/provisioning_class.go @@ -81,13 +81,13 @@ func (o *bestEffortAtomicProvClass) Provision( if len(unschedulablePods) == 0 { return &status.ScaleUpStatus{Result: status.ScaleUpNotTried}, nil } - pr, err := provreqclient.ProvisioningRequestForPods(o.client, unschedulablePods) - if err != nil { - return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, err.Error())) - } - if pr.Spec.ProvisioningClassName != v1beta1.ProvisioningClassBestEffortAtomicScaleUp { + prs := provreqclient.ProvisioningRequestsForPods(o.client, unschedulablePods) + prs = provreqclient.FilterOutProvisioningClass(prs, v1beta1.ProvisioningClassBestEffortAtomicScaleUp) + if len(prs) == 0 { return &status.ScaleUpStatus{Result: status.ScaleUpNotTried}, nil } + // Pick 1 ProvisioningRequest. + pr := prs[0] o.context.ClusterSnapshot.Fork() defer o.context.ClusterSnapshot.Revert() diff --git a/cluster-autoscaler/provisioningrequest/checkcapacity/provisioningclass.go b/cluster-autoscaler/provisioningrequest/checkcapacity/provisioningclass.go index d535d433a648..c87f73bb73fe 100644 --- a/cluster-autoscaler/provisioningrequest/checkcapacity/provisioningclass.go +++ b/cluster-autoscaler/provisioningrequest/checkcapacity/provisioningclass.go @@ -73,14 +73,14 @@ func (o *checkCapacityProvClass) Provision( if len(unschedulablePods) == 0 { return &status.ScaleUpStatus{Result: status.ScaleUpNotTried}, nil } - pr, err := provreqclient.ProvisioningRequestForPods(o.client, unschedulablePods) - if err != nil { - return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, err.Error())) - } - if pr.Spec.ProvisioningClassName != v1beta1.ProvisioningClassCheckCapacity { + + prs := provreqclient.ProvisioningRequestsForPods(o.client, unschedulablePods) + prs = provreqclient.FilterOutProvisioningClass(prs, v1beta1.ProvisioningClassCheckCapacity) + if len(prs) == 0 { return &status.ScaleUpStatus{Result: status.ScaleUpNotTried}, nil } - + // Pick 1 ProvisioningRequest. + pr := prs[0] o.context.ClusterSnapshot.Fork() defer o.context.ClusterSnapshot.Revert() diff --git a/cluster-autoscaler/provisioningrequest/provreqclient/client.go b/cluster-autoscaler/provisioningrequest/provreqclient/client.go index f7ead2d5b3ca..d8963a4d9978 100644 --- a/cluster-autoscaler/provisioningrequest/provreqclient/client.go +++ b/cluster-autoscaler/provisioningrequest/provreqclient/client.go @@ -178,30 +178,31 @@ func newPodTemplatesLister(client *kubernetes.Clientset, stopChannel <-chan stru return podTemplLister, nil } -// ProvisioningRequestForPods check that all pods belong to one ProvisioningRequest and return it. -func ProvisioningRequestForPods(client *ProvisioningRequestClient, unschedulablePods []*apiv1.Pod) (*provreqwrapper.ProvisioningRequest, error) { +// ProvisioningRequestsForPods check that all pods belong to one ProvisioningRequest and return it. +func ProvisioningRequestsForPods(client *ProvisioningRequestClient, unschedulablePods []*apiv1.Pod) []*provreqwrapper.ProvisioningRequest { + prMap := make(map[string]*provreqwrapper.ProvisioningRequest) + prList := []*provreqwrapper.ProvisioningRequest{} if len(unschedulablePods) == 0 { - return nil, fmt.Errorf("empty unschedulablePods list") - } - if unschedulablePods[0].OwnerReferences == nil || len(unschedulablePods[0].OwnerReferences) == 0 { - return nil, fmt.Errorf("pod %s has no OwnerReference", unschedulablePods[0].Name) - } - provReq, err := client.ProvisioningRequest(unschedulablePods[0].Namespace, unschedulablePods[0].OwnerReferences[0].Name) - if err != nil { - return nil, fmt.Errorf("failed retrive ProvisioningRequest from unscheduled pods, err: %v", err) + return prList } for _, pod := range unschedulablePods { - if pod.Namespace != unschedulablePods[0].Namespace { - return nil, fmt.Errorf("pods %s and %s are from different namespaces", pod.Name, unschedulablePods[0].Name) - } if pod.OwnerReferences == nil || len(pod.OwnerReferences) == 0 { - return nil, fmt.Errorf("pod %s has no OwnerReference", pod.Name) + klog.Errorf("pod %s has no OwnerReference", pod.Name) + continue + } + provReq, err := client.ProvisioningRequest(pod.Namespace, pod.OwnerReferences[0].Name) + if err != nil { + klog.Errorf("failed to retrieve ProvisioningRequest from unschedulable pod, err: %v", err) + continue } - if pod.OwnerReferences[0].Name != unschedulablePods[0].OwnerReferences[0].Name { - return nil, fmt.Errorf("pods %s and %s have different OwnerReference", pod.Name, unschedulablePods[0].Name) + if _, found := prMap[provReq.Name]; !found { + prMap[provReq.Name] = provReq } } - return provReq, nil + for _, pr := range prMap { + prList = append(prList, pr) + } + return prList } // DeleteProvisioningRequest deletes the given ProvisioningRequest CR using the ProvisioningRequestInterface and returns an error in case of failure. @@ -216,3 +217,14 @@ func (c *ProvisioningRequestClient) DeleteProvisioningRequest(pr *v1beta1.Provis klog.V(4).Infof("Deleted ProvisioningRequest %s/%s", pr.Namespace, pr.Name) return nil } + +// FilterOutProvisioningClass filters out ProvReqs that belongs to certain Provisioning Class +func FilterOutProvisioningClass(prList []*provreqwrapper.ProvisioningRequest, class string) []*provreqwrapper.ProvisioningRequest { + newPrList := []*provreqwrapper.ProvisioningRequest{} + for _, pr := range prList { + if pr.Spec.ProvisioningClassName == class { + newPrList = append(newPrList, pr) + } + } + return newPrList +} diff --git a/cluster-autoscaler/provisioningrequest/provreqclient/client_test.go b/cluster-autoscaler/provisioningrequest/provreqclient/client_test.go index 15f9e10c3928..0ec9a610c62f 100644 --- a/cluster-autoscaler/provisioningrequest/provreqclient/client_test.go +++ b/cluster-autoscaler/provisioningrequest/provreqclient/client_test.go @@ -49,7 +49,7 @@ func TestFetchPodTemplates(t *testing.T) { } } -func TestProvisioningRequestForPods(t *testing.T) { +func TestProvisioningRequestsForPods(t *testing.T) { checkCapacityProvReq := provreqwrapper.BuildTestProvisioningRequest("ns", "check-capacity", "1m", "100", "", int32(100), false, time.Now(), v1beta1.ProvisioningClassCheckCapacity) customProvReq := provreqwrapper.BuildTestProvisioningRequest("ns", "custom", "1m", "100", "", int32(100), false, time.Now(), "custom") checkCapacityPods, _ := pods.PodsForProvisioningRequest(checkCapacityProvReq) @@ -57,55 +57,41 @@ func TestProvisioningRequestForPods(t *testing.T) { regularPod := BuildTestPod("p1", 600, 100) client := NewFakeProvisioningRequestClient(context.Background(), t, checkCapacityProvReq, customProvReq) testCases := []struct { - name string - pods []*apiv1.Pod - className string - err bool - pr *provreqwrapper.ProvisioningRequest + name string + pods []*apiv1.Pod + err bool + prs []*provreqwrapper.ProvisioningRequest }{ { - name: "no pods", - pods: []*apiv1.Pod{}, - className: "some-class", - err: true, + name: "no pods", + pods: []*apiv1.Pod{}, }, { - name: "pods from one Provisioning Class", - pods: checkCapacityPods, - className: v1beta1.ProvisioningClassCheckCapacity, - pr: checkCapacityProvReq, + name: "pods from one Provisioning Class", + pods: checkCapacityPods, + prs: []*provreqwrapper.ProvisioningRequest{checkCapacityProvReq}, }, { - name: "pods from different Provisioning Classes", - pods: append(checkCapacityPods, customProvReqPods...), - className: v1beta1.ProvisioningClassCheckCapacity, - err: true, + name: "pods from different Provisioning Classes", + pods: append(checkCapacityPods, customProvReqPods...), + prs: []*provreqwrapper.ProvisioningRequest{checkCapacityProvReq, customProvReq}, }, { - name: "regular pod", - pods: []*apiv1.Pod{regularPod}, - className: v1beta1.ProvisioningClassCheckCapacity, - err: true, + name: "regular pod", + pods: []*apiv1.Pod{regularPod}, }, { - name: "provreq pods and regular pod", - pods: append(checkCapacityPods, regularPod), - className: v1beta1.ProvisioningClassCheckCapacity, - err: true, + name: "provreq pods and regular pod", + pods: append(checkCapacityPods, regularPod), + prs: []*provreqwrapper.ProvisioningRequest{checkCapacityProvReq}, }, } for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - pr, err := ProvisioningRequestForPods(client, tc.pods) - if tc.err { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, pr, tc.pr) - assert.Equal(t, pr.Spec.ProvisioningClassName, tc.className) - } + got := ProvisioningRequestsForPods(client, tc.pods) + assert.ElementsMatch(t, got, tc.prs) }) } }