Do not fail if multiple ProvReqs are injected

This commit is contained in:
Yaroslava Serdiuk 2024-07-02 09:16:35 +00:00
parent c8a016d5e5
commit 2824fc9b16
4 changed files with 60 additions and 62 deletions

View File

@ -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()

View File

@ -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()

View File

@ -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
}
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)
provReq, err := client.ProvisioningRequest(pod.Namespace, pod.OwnerReferences[0].Name)
if err != nil {
klog.Errorf("failed retrive ProvisioningRequest from unscheduled pods, err: %v", err)
continue
}
if prMap[provReq.Name] == nil {
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
}

View File

@ -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)
})
}
}