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
DRA: test for structured parameters #123938
Changes from all commits
f149d6d
cf8fffa
95136db
4126e37
607261e
458e227
6f5696b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -293,6 +293,31 @@ func TestSyncHandler(t *testing.T) { | |
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "clear-reserved-delayed-allocation-structured", | ||
pods: []*v1.Pod{}, | ||
key: claimKey(testClaimReserved), | ||
claims: []*resourcev1alpha2.ResourceClaim{structuredParameters(testClaimReserved)}, | ||
expectedClaims: func() []resourcev1alpha2.ResourceClaim { | ||
claim := testClaimAllocated.DeepCopy() | ||
claim.Finalizers = []string{} | ||
claim.Status.Allocation = nil | ||
return []resourcev1alpha2.ResourceClaim{*claim} | ||
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "dont-clear-reserved-delayed-allocation-structured", | ||
pods: []*v1.Pod{testPodWithResource}, | ||
key: claimKey(testClaimReserved), | ||
claims: func() []*resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimReserved) | ||
claim = reserveClaim(claim, otherTestPod) | ||
return []*resourcev1alpha2.ResourceClaim{claim} | ||
}(), | ||
expectedClaims: []resourcev1alpha2.ResourceClaim{*structuredParameters(testClaimReserved)}, | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "clear-reserved-immediate-allocation", | ||
pods: []*v1.Pod{}, | ||
|
@@ -309,6 +334,62 @@ func TestSyncHandler(t *testing.T) { | |
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "clear-reserved-immediate-allocation-structured", | ||
pods: []*v1.Pod{}, | ||
key: claimKey(testClaimReserved), | ||
claims: func() []*resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimReserved.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
return []*resourcev1alpha2.ResourceClaim{claim} | ||
}(), | ||
expectedClaims: func() []resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimAllocated.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
return []resourcev1alpha2.ResourceClaim{*claim} | ||
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "clear-reserved-immediate-allocation-structured-deleted", | ||
pods: []*v1.Pod{}, | ||
key: claimKey(testClaimReserved), | ||
claims: func() []*resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimReserved.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
claim.DeletionTimestamp = &metav1.Time{} | ||
return []*resourcev1alpha2.ResourceClaim{claim} | ||
}(), | ||
expectedClaims: func() []resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimAllocated.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
claim.DeletionTimestamp = &metav1.Time{} | ||
claim.Finalizers = []string{} | ||
claim.Status.Allocation = nil | ||
return []resourcev1alpha2.ResourceClaim{*claim} | ||
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "immediate-allocation-structured-deleted", | ||
pods: []*v1.Pod{}, | ||
key: claimKey(testClaimReserved), | ||
claims: func() []*resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimAllocated.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
claim.DeletionTimestamp = &metav1.Time{} | ||
return []*resourcev1alpha2.ResourceClaim{claim} | ||
}(), | ||
expectedClaims: func() []resourcev1alpha2.ResourceClaim { | ||
claim := structuredParameters(testClaimAllocated.DeepCopy()) | ||
claim.Spec.AllocationMode = resourcev1alpha2.AllocationModeImmediate | ||
claim.DeletionTimestamp = &metav1.Time{} | ||
claim.Finalizers = []string{} | ||
claim.Status.Allocation = nil | ||
return []resourcev1alpha2.ResourceClaim{*claim} | ||
}(), | ||
expectedMetrics: expectedMetrics{0, 0}, | ||
}, | ||
{ | ||
name: "clear-reserved-when-done-delayed-allocation", | ||
pods: func() []*v1.Pod { | ||
|
@@ -546,6 +627,14 @@ func allocateClaim(claim *resourcev1alpha2.ResourceClaim) *resourcev1alpha2.Reso | |
return claim | ||
} | ||
|
||
func structuredParameters(claim *resourcev1alpha2.ResourceClaim) *resourcev1alpha2.ResourceClaim { | ||
claim = claim.DeepCopy() | ||
// As far the controller is concerned, a claim was allocated by us if it has | ||
// this finalizer. For testing we don't need to update the allocation result. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a warning in the API comment for the Finalizer constant saying that third-party drivers shouldn't use this finalizer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add that in a separate PR for the API. I have another comment update pending. |
||
claim.Finalizers = append(claim.Finalizers, resourcev1alpha2.Finalizer) | ||
return claim | ||
} | ||
|
||
func reserveClaim(claim *resourcev1alpha2.ResourceClaim, pod *v1.Pod) *resourcev1alpha2.ResourceClaim { | ||
claim = claim.DeepCopy() | ||
claim.Status.ReservedFor = append(claim.Status.ReservedFor, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1209,7 +1209,7 @@ func (pl *dynamicResources) PostFilter(ctx context.Context, cs *framework.CycleS | |
// Then we can simply clear the allocation. Once the | ||
// claim informer catches up, the controllers will | ||
// be notified about this change. | ||
clearAllocation := state.informationsForClaim[index].controller != nil | ||
clearAllocation := state.informationsForClaim[index].structuredParameters | ||
|
||
// Before we tell a driver to deallocate a claim, we | ||
// have to stop telling it to allocate. Otherwise, | ||
|
@@ -1228,6 +1228,7 @@ func (pl *dynamicResources) PostFilter(ctx context.Context, cs *framework.CycleS | |
claim := claim.DeepCopy() | ||
claim.Status.ReservedFor = nil | ||
if clearAllocation { | ||
claim.Status.DriverName = "" | ||
claim.Status.Allocation = nil | ||
} else { | ||
claim.Status.DeallocationRequested = true | ||
|
@@ -1410,7 +1411,11 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat | |
} | ||
state.informationsForClaim[index].allocation = allocation | ||
state.informationsForClaim[index].allocationDriverName = driverName | ||
// Strictly speaking, we don't need to store the full modified object. | ||
// The allocation would be enough. The full object is useful for | ||
// debugging and testing, so let's make it realistic. | ||
claim = claim.DeepCopy() | ||
claim.Finalizers = append(claim.Finalizers, resourcev1alpha2.Finalizer) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this all happen in memory? I don't suppose this object is persisted to the API, is it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All in memory. It's debatable whether it should store the whole "expected" claim at all. It's consistent, but not really required. |
||
claim.Status.DriverName = driverName | ||
claim.Status.Allocation = allocation | ||
pl.inFlightAllocations.Store(claim.UID, claim) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are missing a test where the allocation isn't cleared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added (but not pushed yet - I'm not sure whether I'll be able finish today).