Skip to content

Commit

Permalink
DRA: remove immediate allocation
Browse files Browse the repository at this point in the history 
As agreed in kubernetes/enhancements#4709, immediate
allocation is one of those features which can be removed because it makes no
sense for structured parameters and the justification for classic DRA is weak.

Kubernetes-commit: de5742ae83c8d77268a7caf5f3b1f418c4a13a84
  • Loading branch information
@pohly @k8s-publishing-bot
pohly authored and k8s-publishing-bot committed Jun 13, 2024
1 parent 4940ebf commit f0aba33
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 197 deletions.
47 changes: 1 addition & 46 deletions controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,48 +515,7 @@ func (ctrl *controller) syncClaim(ctx context.Context, claim *resourceapi.Resour
logger.V(5).Info("ResourceClaim is allocated")
return nil
}
if claim.Spec.AllocationMode != resourceapi.AllocationModeImmediate {
logger.V(5).Info("ResourceClaim waiting for first consumer")
return nil
}

// We need the ResourceClass to determine whether we should allocate it.
class, err := ctrl.rcLister.Get(claim.Spec.ResourceClassName)
if err != nil {
return err
}
if class.DriverName != ctrl.name {
// Not ours *at the moment*. This can change, so requeue and
// check again. We could trigger a faster check when the
// ResourceClass changes, but that shouldn't occur much in
// practice and thus isn't worth the effort.
//
// We use exponential backoff because it is unlikely that
// the ResourceClass changes much.
logger.V(5).Info("ResourceClaim is handled by other driver", "driver", class.DriverName)
return errRequeue
}

// Check parameters. Do not record event to Claim if its parameters are invalid,
// syncKey will record the error.
claimParameters, classParameters, err := ctrl.getParameters(ctx, claim, class, false)
if err != nil {
return err
}

claimAllocations := claimAllocations{&ClaimAllocation{
Claim: claim,
ClaimParameters: claimParameters,
Class: class,
ClassParameters: classParameters,
}}

ctrl.allocateClaims(ctx, claimAllocations, "", nil)

if claimAllocations[0].Error != nil {
return fmt.Errorf("allocate: %v", claimAllocations[0].Error)
}

logger.V(5).Info("ResourceClaim waiting for first consumer")
return nil
}

Expand Down Expand Up @@ -678,10 +637,6 @@ func (ctrl *controller) checkPodClaim(ctx context.Context, pod *v1.Pod, podClaim
return nil, err
}
}
if claim.Spec.AllocationMode != resourceapi.AllocationModeWaitForFirstConsumer {
// Nothing to do for it as part of pod scheduling.
return nil, nil
}
if claim.Status.Allocation != nil {
// Already allocated, class and parameter are not needed and nothing
// need to be done for the claim either.
Expand Down
198 changes: 47 additions & 151 deletions controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ func TestController(t *testing.T) {
}
claim := createClaim(claimName, claimNamespace, className)
otherClaim := createClaim(claimName, claimNamespace, otherClassName)
delayedClaim := claim.DeepCopy()
delayedClaim.Spec.AllocationMode = resourceapi.AllocationModeWaitForFirstConsumer
podName := "pod"
podKey := "schedulingCtx:default/pod"
pod := createPod(podName, claimNamespace, nil)
Expand Down Expand Up @@ -148,94 +146,6 @@ func TestController(t *testing.T) {
classes: classes,
claim: otherClaim,
expectedClaim: otherClaim,
expectedError: errRequeue.Error(), // class might change
},
// Immediate allocation:
// deletion time stamp set, our finalizer set, not allocated -> remove finalizer
"immediate-deleted-finalizer-removal": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
driver: m.expectDeallocate(map[string]error{claimName: nil}),
expectedClaim: withDeletionTimestamp(claim),
},
// deletion time stamp set, our finalizer set, not allocated, stopping fails -> requeue
"immediate-deleted-finalizer-stop-failure": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
driver: m.expectDeallocate(map[string]error{claimName: errors.New("fake error")}),
expectedClaim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
expectedError: "stop allocation: fake error",
},
// deletion time stamp set, other finalizer set, not allocated -> do nothing
"immediate-deleted-finalizer-no-removal": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(claim), otherFinalizer),
expectedClaim: withFinalizer(withDeletionTimestamp(claim), otherFinalizer),
},
// deletion time stamp set, finalizer set, allocated -> deallocate
"immediate-deleted-allocated": {
key: claimKey,
classes: classes,
claim: withAllocate(withDeletionTimestamp(claim)),
driver: m.expectDeallocate(map[string]error{claimName: nil}),
expectedClaim: withDeletionTimestamp(claim),
},
// deletion time stamp set, finalizer set, allocated, deallocation fails -> requeue
"immediate-deleted-deallocate-failure": {
key: claimKey,
classes: classes,
claim: withAllocate(withDeletionTimestamp(claim)),
driver: m.expectDeallocate(map[string]error{claimName: errors.New("fake error")}),
expectedClaim: withAllocate(withDeletionTimestamp(claim)),
expectedError: "deallocate: fake error",
},
// deletion time stamp set, finalizer not set -> do nothing
"immediate-deleted-no-finalizer": {
key: claimKey,
classes: classes,
claim: withDeletionTimestamp(claim),
expectedClaim: withDeletionTimestamp(claim),
},
// not deleted, not allocated, no finalizer -> add finalizer, allocate
"immediate-do-allocation": {
key: claimKey,
classes: classes,
claim: claim,
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
expectClaimParameters(map[string]interface{}{claimName: 2}).
expectAllocate(map[string]allocate{claimName: {allocResult: &allocation, allocErr: nil}}),
expectedClaim: withAllocate(claim),
},
// not deleted, not allocated, finalizer -> allocate
"immediate-continue-allocation": {
key: claimKey,
classes: classes,
claim: withFinalizer(claim, ourFinalizer),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
expectClaimParameters(map[string]interface{}{claimName: 2}).
expectAllocate(map[string]allocate{claimName: {allocResult: &allocation, allocErr: nil}}),
expectedClaim: withAllocate(claim),
},
// not deleted, not allocated, finalizer, fail allocation -> requeue
"immediate-fail-allocation": {
key: claimKey,
classes: classes,
claim: withFinalizer(claim, ourFinalizer),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
expectClaimParameters(map[string]interface{}{claimName: 2}).
expectAllocate(map[string]allocate{claimName: {allocErr: errors.New("fake error")}}),
expectedClaim: withFinalizer(claim, ourFinalizer),
expectedError: "allocate: fake error",
},
// not deleted, allocated -> do nothing
"immediate-allocated-nop": {
key: claimKey,
classes: classes,
claim: withAllocate(claim),
expectedClaim: withAllocate(claim),
},

// not deleted, reallocate -> deallocate
Expand All @@ -257,62 +167,60 @@ func TestController(t *testing.T) {
expectedError: "deallocate: fake error",
},

// Delayed allocation is similar in some cases, but not quite
// the same.
// deletion time stamp set, our finalizer set, not allocated -> remove finalizer
"delayed-deleted-finalizer-removal": {
"deleted-finalizer-removal": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(delayedClaim), ourFinalizer),
claim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
driver: m.expectDeallocate(map[string]error{claimName: nil}),
expectedClaim: withDeletionTimestamp(delayedClaim),
expectedClaim: withDeletionTimestamp(claim),
},
// deletion time stamp set, our finalizer set, not allocated, stopping fails -> requeue
"delayed-deleted-finalizer-stop-failure": {
"deleted-finalizer-stop-failure": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(delayedClaim), ourFinalizer),
claim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
driver: m.expectDeallocate(map[string]error{claimName: errors.New("fake error")}),
expectedClaim: withFinalizer(withDeletionTimestamp(delayedClaim), ourFinalizer),
expectedClaim: withFinalizer(withDeletionTimestamp(claim), ourFinalizer),
expectedError: "stop allocation: fake error",
},
// deletion time stamp set, other finalizer set, not allocated -> do nothing
"delayed-deleted-finalizer-no-removal": {
"deleted-finalizer-no-removal": {
key: claimKey,
classes: classes,
claim: withFinalizer(withDeletionTimestamp(delayedClaim), otherFinalizer),
expectedClaim: withFinalizer(withDeletionTimestamp(delayedClaim), otherFinalizer),
claim: withFinalizer(withDeletionTimestamp(claim), otherFinalizer),
expectedClaim: withFinalizer(withDeletionTimestamp(claim), otherFinalizer),
},
// deletion time stamp set, finalizer set, allocated -> deallocate
"delayed-deleted-allocated": {
"deleted-allocated": {
key: claimKey,
classes: classes,
claim: withAllocate(withDeletionTimestamp(delayedClaim)),
claim: withAllocate(withDeletionTimestamp(claim)),
driver: m.expectDeallocate(map[string]error{claimName: nil}),
expectedClaim: withDeletionTimestamp(delayedClaim),
expectedClaim: withDeletionTimestamp(claim),
},
// deletion time stamp set, finalizer set, allocated, deallocation fails -> requeue
"delayed-deleted-deallocate-failure": {
"deleted-deallocate-failure": {
key: claimKey,
classes: classes,
claim: withAllocate(withDeletionTimestamp(delayedClaim)),
claim: withAllocate(withDeletionTimestamp(claim)),
driver: m.expectDeallocate(map[string]error{claimName: errors.New("fake error")}),
expectedClaim: withAllocate(withDeletionTimestamp(delayedClaim)),
expectedClaim: withAllocate(withDeletionTimestamp(claim)),
expectedError: "deallocate: fake error",
},
// deletion time stamp set, finalizer not set -> do nothing
"delayed-deleted-no-finalizer": {
"deleted-no-finalizer": {
key: claimKey,
classes: classes,
claim: withDeletionTimestamp(delayedClaim),
expectedClaim: withDeletionTimestamp(delayedClaim),
claim: withDeletionTimestamp(claim),
expectedClaim: withDeletionTimestamp(claim),
},
// waiting for first consumer -> do nothing
"delayed-pending": {
"pending": {
key: claimKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
},

// pod with no claims -> shouldn't occur, check again anyway
Expand All @@ -324,34 +232,23 @@ func TestController(t *testing.T) {
expectedError: errPeriodic.Error(),
},

// pod with immediate allocation and selected node -> shouldn't occur, check again in case that claim changes
"pod-immediate": {
// no potential nodes -> shouldn't occur
"no-nodes": {
key: podKey,
classes: classes,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withSelectedNode(podSchedulingCtx),
expectedSchedulingCtx: withSelectedNode(podSchedulingCtx),
expectedError: errPeriodic.Error(),
},

// pod with delayed allocation, no potential nodes -> shouldn't occur
"pod-delayed-no-nodes": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
pod: podWithClaim,
schedulingCtx: podSchedulingCtx,
expectedSchedulingCtx: podSchedulingCtx,
},

// pod with delayed allocation, potential nodes -> provide unsuitable nodes
"pod-delayed-info": {
// potential nodes -> provide unsuitable nodes
"info": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withPotentialNodes(podSchedulingCtx),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
Expand All @@ -361,23 +258,23 @@ func TestController(t *testing.T) {
expectedError: errPeriodic.Error(),
},

// pod with delayed allocation, potential nodes, selected node, missing class -> failure
"pod-delayed-missing-class": {
// potential nodes, selected node, missing class -> failure
"missing-class": {
key: podKey,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withSelectedNode(withPotentialNodes(podSchedulingCtx)),
expectedSchedulingCtx: withSelectedNode(withPotentialNodes(podSchedulingCtx)),
expectedError: `pod claim my-pod-claim: resourceclass.resource.k8s.io "mock-class" not found`,
},

// pod with delayed allocation, potential nodes, selected node -> allocate
"pod-delayed-allocate": {
// potential nodes, selected node -> allocate
"allocate": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: withReservedFor(withAllocate(delayedClaim), pod),
claim: claim,
expectedClaim: withReservedFor(withAllocate(claim), pod),
pod: podWithClaim,
schedulingCtx: withSelectedNode(withPotentialNodes(podSchedulingCtx)),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
Expand All @@ -387,12 +284,12 @@ func TestController(t *testing.T) {
expectedSchedulingCtx: withUnsuitableNodes(withSelectedNode(withPotentialNodes(podSchedulingCtx))),
expectedError: errPeriodic.Error(),
},
// pod with delayed allocation, potential nodes, selected node, all unsuitable -> update unsuitable nodes
"pod-selected-is-potential-node": {
// potential nodes, selected node, all unsuitable -> update unsuitable nodes
"is-potential-node": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withPotentialNodes(withSelectedNode(withPotentialNodes(podSchedulingCtx))),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
Expand All @@ -401,12 +298,12 @@ func TestController(t *testing.T) {
expectedSchedulingCtx: withSpecificUnsuitableNodes(withSelectedNode(withPotentialNodes(podSchedulingCtx)), potentialNodes),
expectedError: errPeriodic.Error(),
},
// pod with delayed allocation, max potential nodes, other selected node, all unsuitable -> update unsuitable nodes with truncation at start
"pod-selected-is-potential-node-truncate-first": {
// max potential nodes, other selected node, all unsuitable -> update unsuitable nodes with truncation at start
"is-potential-node-truncate-first": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withSpecificPotentialNodes(withSelectedNode(withSpecificPotentialNodes(podSchedulingCtx, maxNodes)), maxNodes),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
Expand All @@ -415,12 +312,12 @@ func TestController(t *testing.T) {
expectedSchedulingCtx: withSpecificUnsuitableNodes(withSelectedNode(withSpecificPotentialNodes(podSchedulingCtx, maxNodes)), append(maxNodes[1:], nodeName)),
expectedError: errPeriodic.Error(),
},
// pod with delayed allocation, max potential nodes, other selected node, all unsuitable (but in reverse order) -> update unsuitable nodes with truncation at end
// max potential nodes, other selected node, all unsuitable (but in reverse order) -> update unsuitable nodes with truncation at end
"pod-selected-is-potential-node-truncate-last": {
key: podKey,
classes: classes,
claim: delayedClaim,
expectedClaim: delayedClaim,
claim: claim,
expectedClaim: claim,
pod: podWithClaim,
schedulingCtx: withSpecificPotentialNodes(withSelectedNode(withSpecificPotentialNodes(podSchedulingCtx, maxNodes)), maxNodes),
driver: m.expectClassParameters(map[string]interface{}{className: 1}).
Expand Down Expand Up @@ -652,7 +549,6 @@ func createClaim(claimName, claimNamespace, className string) *resourceapi.Resou
},
Spec: resourceapi.ResourceClaimSpec{
ResourceClassName: className,
AllocationMode: resourceapi.AllocationModeImmediate,
},
}
}
Expand Down

0 comments on commit f0aba33

Please sign in to comment.