Skip to content

Commit 1025aa2

Browse files
jmdealjmdeal
committed
testing + misc fixes
1 parent 2619399 commit 1025aa2

File tree

7 files changed

+150
-31
lines changed

7 files changed

+150
-31
lines changed

‎pkg/cloudprovider/suite_test.go

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -915,11 +915,17 @@ var _ = Describe("CloudProvider", func() {
915915
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
916916
InstanceType: "m5.large",
917917
OwnerID: "012345678901",
918+
State: v1.CapacityReservationStateActive,
919+
ReservationType: v1.CapacityReservationTypeDefault,
918920
},
919921
}
920922
setReservationID := func(id string) {
921923
out := awsEnv.EC2API.DescribeInstancesBehavior.Output.Clone()
924+
out.Reservations[0].Instances[0].SpotInstanceRequestId = nil
922925
out.Reservations[0].Instances[0].CapacityReservationId = lo.ToPtr(id)
926+
out.Reservations[0].Instances[0].CapacityReservationSpecification = &ec2types.CapacityReservationSpecificationResponse{
927+
CapacityReservationPreference: ec2types.CapacityReservationPreferenceCapacityReservationsOnly,
928+
}
923929
awsEnv.EC2API.DescribeInstancesBehavior.Output.Set(out)
924930
}
925931
setReservationID("cr-foo")
@@ -1414,25 +1420,30 @@ var _ = Describe("CloudProvider", func() {
14141420
})
14151421
})
14161422
Context("Capacity Reservations", func() {
1417-
var reservationID string
1423+
const reservationCapacity = 10
1424+
var crs []ec2types.CapacityReservation
14181425
BeforeEach(func() {
1419-
reservationID = "cr-m5.large-1a-1"
1420-
cr := ec2types.CapacityReservation{
1421-
AvailabilityZone: lo.ToPtr("test-zone-1a"),
1422-
InstanceType: lo.ToPtr("m5.large"),
1423-
OwnerId: lo.ToPtr("012345678901"),
1424-
InstanceMatchCriteria: ec2types.InstanceMatchCriteriaTargeted,
1425-
CapacityReservationId: lo.ToPtr(reservationID),
1426-
AvailableInstanceCount: lo.ToPtr[int32](10),
1427-
State: ec2types.CapacityReservationStateActive,
1426+
crs = lo.Map(v1.CapacityReservationType("").Values(), func(crt v1.CapacityReservationType, _ int) ec2types.CapacityReservation {
1427+
return ec2types.CapacityReservation{
1428+
AvailabilityZone: lo.ToPtr("test-zone-1a"),
1429+
InstanceType: lo.ToPtr("m5.large"),
1430+
OwnerId: lo.ToPtr("012345678901"),
1431+
InstanceMatchCriteria: ec2types.InstanceMatchCriteriaTargeted,
1432+
CapacityReservationId: lo.ToPtr(fmt.Sprintf("cr-m5.large-1a-%s", string(crt))),
1433+
AvailableInstanceCount: lo.ToPtr[int32](reservationCapacity),
1434+
State: ec2types.CapacityReservationStateActive,
1435+
ReservationType: ec2types.CapacityReservationType(crt),
1436+
}
1437+
})
1438+
for _, cr := range crs {
1439+
awsEnv.CapacityReservationProvider.SetAvailableInstanceCount(*cr.CapacityReservationId, 10)
14281440
}
1429-
awsEnv.CapacityReservationProvider.SetAvailableInstanceCount(reservationID, 10)
14301441
awsEnv.EC2API.DescribeCapacityReservationsOutput.Set(&ec2.DescribeCapacityReservationsOutput{
1431-
CapacityReservations: []ec2types.CapacityReservation{cr},
1442+
CapacityReservations: crs,
1443+
})
1444+
nodeClass.Status.CapacityReservations = lo.Map(crs, func(cr ec2types.CapacityReservation, _ int) v1.CapacityReservation {
1445+
return lo.Must(v1.CapacityReservationFromEC2(awsEnv.Clock, &cr))
14321446
})
1433-
nodeClass.Status.CapacityReservations = []v1.CapacityReservation{
1434-
lo.Must(v1.CapacityReservationFromEC2(fakeClock, &cr)),
1435-
}
14361447
nodePool.Spec.Template.Spec.Requirements = []karpv1.NodeSelectorRequirementWithMinValues{{NodeSelectorRequirement: corev1.NodeSelectorRequirement{
14371448
Key: karpv1.CapacityTypeLabelKey,
14381449
Operator: corev1.NodeSelectorOpIn,
@@ -1444,7 +1455,9 @@ var _ = Describe("CloudProvider", func() {
14441455
ExpectApplied(ctx, env.Client, nodePool, nodeClass, pod)
14451456
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
14461457
ExpectScheduled(ctx, env.Client, pod)
1447-
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(reservationID)).To(Equal(9))
1458+
ncs := ExpectNodeClaims(ctx, env.Client)
1459+
Expect(ncs).To(HaveLen(1))
1460+
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(ncs[0].Labels[corecloudprovider.ReservationIDLabel])).To(Equal(9))
14481461
})
14491462
It("should mark capacity reservations as terminated", func() {
14501463
pod := coretest.UnschedulablePod()
@@ -1457,24 +1470,38 @@ var _ = Describe("CloudProvider", func() {
14571470
// Attempt the first delete - since the instance still exists we shouldn't increment the availability count
14581471
err := cloudProvider.Delete(ctx, ncs[0])
14591472
Expect(corecloudprovider.IsNodeClaimNotFoundError(err)).To(BeFalse())
1460-
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(reservationID)).To(Equal(9))
1473+
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(ncs[0].Labels[corecloudprovider.ReservationIDLabel])).To(Equal(9))
14611474

14621475
// Attempt again after clearing the instance from the EC2 output. Now that we get a NotFound error, expect
14631476
// availability to be incremented.
14641477
awsEnv.EC2API.DescribeInstancesBehavior.Output.Set(&ec2.DescribeInstancesOutput{})
14651478
err = cloudProvider.Delete(ctx, ncs[0])
14661479
Expect(corecloudprovider.IsNodeClaimNotFoundError(err)).To(BeTrue())
1467-
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(reservationID)).To(Equal(10))
1468-
})
1469-
It("should include capacity reservation labels", func() {
1470-
pod := coretest.UnschedulablePod()
1471-
ExpectApplied(ctx, env.Client, nodePool, nodeClass, pod)
1472-
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
1473-
ExpectScheduled(ctx, env.Client, pod)
1474-
ncs := ExpectNodeClaims(ctx, env.Client)
1475-
Expect(ncs).To(HaveLen(1))
1476-
Expect(ncs[0].Labels).To(HaveKeyWithValue(karpv1.CapacityTypeLabelKey, karpv1.CapacityTypeReserved))
1477-
Expect(ncs[0].Labels).To(HaveKeyWithValue(corecloudprovider.ReservationIDLabel, reservationID))
1480+
Expect(awsEnv.CapacityReservationProvider.GetAvailableInstanceCount(ncs[0].Labels[corecloudprovider.ReservationIDLabel])).To(Equal(10))
14781481
})
1482+
DescribeTable(
1483+
"should include capacity reservation labels",
1484+
func(crt v1.CapacityReservationType) {
1485+
pod := coretest.UnschedulablePod(coretest.PodOptions{
1486+
NodeSelector: map[string]string{
1487+
v1.LabelCapacityReservationType: string(crt),
1488+
},
1489+
})
1490+
ExpectApplied(ctx, env.Client, nodePool, nodeClass, pod)
1491+
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
1492+
ExpectScheduled(ctx, env.Client, pod)
1493+
ncs := ExpectNodeClaims(ctx, env.Client)
1494+
Expect(ncs).To(HaveLen(1))
1495+
Expect(ncs[0].Labels).To(HaveKeyWithValue(karpv1.CapacityTypeLabelKey, karpv1.CapacityTypeReserved))
1496+
Expect(ncs[0].Labels).To(HaveKeyWithValue(corecloudprovider.ReservationIDLabel, *lo.Must(lo.Find(crs, func(cr ec2types.CapacityReservation) bool {
1497+
return string(cr.ReservationType) == string(crt)
1498+
1499+
})).CapacityReservationId))
1500+
Expect(ncs[0].Labels).To(HaveKeyWithValue(v1.LabelCapacityReservationType, string(crt)))
1501+
},
1502+
lo.Map(v1.CapacityReservationType("").Values(), func(crt v1.CapacityReservationType, _ int) TableEntry {
1503+
return Entry(fmt.Sprintf("when the capacity reservation type is %q", string(crt)), crt)
1504+
}),
1505+
)
14791506
})
14801507
})

‎pkg/controllers/capacityreservation/capacitytype/suite_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ var _ = Describe("Capacity Reservation Capacity Type Controller", func() {
9191
},
9292
InstanceId: lo.ToPtr(fake.InstanceID()),
9393
CapacityReservationId: &reservationID,
94+
CapacityReservationSpecification: &ec2types.CapacityReservationSpecificationResponse{
95+
CapacityReservationPreference: ec2types.CapacityReservationPreferenceCapacityReservationsOnly,
96+
},
9497
Placement: &ec2types.Placement{
9598
AvailabilityZone: lo.ToPtr("test-zone-1a"),
9699
},

‎pkg/controllers/capacityreservation/expiration/controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"sigs.k8s.io/karpenter/pkg/utils/nodeclaim"
4040

4141
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
42+
4243
v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
4344
"github.com/aws/karpenter-provider-aws/pkg/providers/capacityreservation"
4445
)

‎pkg/controllers/capacityreservation/expiration/suite_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929

3030
"github.com/aws/aws-sdk-go-v2/service/ec2"
3131
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
32+
3233
"github.com/aws/karpenter-provider-aws/pkg/apis"
3334
v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
3435
"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
@@ -55,7 +56,7 @@ var controller *expiration.Controller
5556
func TestAWS(t *testing.T) {
5657
ctx = TestContextWithLogger(t)
5758
RegisterFailHandler(Fail)
58-
RunSpecs(t, "SSM Invalidation Controller")
59+
RunSpecs(t, "CapacityReservationExpiration")
5960
}
6061

6162
var _ = BeforeSuite(func() {
@@ -75,6 +76,16 @@ var _ = AfterSuite(func() {
7576
Expect(env.Stop()).To(Succeed(), "Failed to stop environment")
7677
})
7778

79+
var _ = BeforeEach(func() {
80+
ctx = coreoptions.ToContext(ctx, coretest.Options(coretest.OptionsFields{FeatureGates: coretest.FeatureGates{ReservedCapacity: lo.ToPtr(true)}}))
81+
ctx = options.ToContext(ctx, test.Options())
82+
awsEnv.Reset()
83+
})
84+
85+
var _ = AfterEach(func() {
86+
ExpectCleanedUp(ctx, env.Client)
87+
})
88+
7889
var _ = Describe("Capacity Reservation Expiration Controller", func() {
7990
BeforeEach(func() {
8091
awsEnv.Clock.SetTime(time.Now())

‎pkg/controllers/metrics/suite_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,20 +277,26 @@ var _ = Describe("MetricsController", func() {
277277
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
278278
InstanceType: "m5.large",
279279
OwnerID: "012345678901",
280+
State: v1.CapacityReservationStateActive,
281+
ReservationType: v1.CapacityReservationTypeDefault,
280282
},
281283
{
282284
AvailabilityZone: "test-zone-1b",
283285
ID: "cr-bar",
284286
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
285287
InstanceType: "m5.large",
286288
OwnerID: "012345678901",
289+
State: v1.CapacityReservationStateActive,
290+
ReservationType: v1.CapacityReservationTypeDefault,
287291
},
288292
{
289293
AvailabilityZone: "test-zone-1a",
290294
ID: "cr-baz",
291295
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
292296
InstanceType: "m5.metal",
293297
OwnerID: "012345678901",
298+
State: v1.CapacityReservationStateActive,
299+
ReservationType: v1.CapacityReservationTypeDefault,
294300
},
295301
}
296302
for _, elem := range nodeClass.Status.CapacityReservations {
@@ -369,20 +375,26 @@ var _ = Describe("MetricsController", func() {
369375
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
370376
InstanceType: "m5.large",
371377
OwnerID: "012345678901",
378+
State: v1.CapacityReservationStateActive,
379+
ReservationType: v1.CapacityReservationTypeDefault,
372380
},
373381
{
374382
AvailabilityZone: "test-zone-1b",
375383
ID: "cr-bar",
376384
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
377385
InstanceType: "m5.large",
378386
OwnerID: "012345678901",
387+
State: v1.CapacityReservationStateActive,
388+
ReservationType: v1.CapacityReservationTypeDefault,
379389
},
380390
{
381391
AvailabilityZone: "test-zone-1a",
382392
ID: "cr-baz",
383393
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
384394
InstanceType: "m5.metal",
385395
OwnerID: "012345678901",
396+
State: v1.CapacityReservationStateActive,
397+
ReservationType: v1.CapacityReservationTypeDefault,
386398
},
387399
}
388400
for _, elem := range nodeClass.Status.CapacityReservations {
@@ -481,20 +493,26 @@ var _ = Describe("MetricsController", func() {
481493
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
482494
InstanceType: "m5.large",
483495
OwnerID: "012345678901",
496+
State: v1.CapacityReservationStateActive,
497+
ReservationType: v1.CapacityReservationTypeDefault,
484498
},
485499
{
486500
AvailabilityZone: "test-zone-1b",
487501
ID: "cr-bar",
488502
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
489503
InstanceType: "m5.large",
490504
OwnerID: "012345678901",
505+
State: v1.CapacityReservationStateActive,
506+
ReservationType: v1.CapacityReservationTypeDefault,
491507
},
492508
{
493509
AvailabilityZone: "test-zone-1a",
494510
ID: "cr-baz",
495511
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
496512
InstanceType: "m5.metal",
497513
OwnerID: "012345678901",
514+
State: v1.CapacityReservationStateActive,
515+
ReservationType: v1.CapacityReservationTypeDefault,
498516
},
499517
}
500518
for _, elem := range nodeClass.Status.CapacityReservations {
@@ -569,20 +587,26 @@ var _ = Describe("MetricsController", func() {
569587
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
570588
InstanceType: "m5.large",
571589
OwnerID: "012345678901",
590+
State: v1.CapacityReservationStateActive,
591+
ReservationType: v1.CapacityReservationTypeDefault,
572592
},
573593
{
574594
AvailabilityZone: "test-zone-1b",
575595
ID: "cr-bar",
576596
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
577597
InstanceType: "m5.large",
578598
OwnerID: "012345678901",
599+
State: v1.CapacityReservationStateActive,
600+
ReservationType: v1.CapacityReservationTypeDefault,
579601
},
580602
{
581603
AvailabilityZone: "test-zone-1a",
582604
ID: "cr-baz",
583605
InstanceMatchCriteria: string(ec2types.InstanceMatchCriteriaTargeted),
584606
InstanceType: "m5.metal",
585607
OwnerID: "012345678901",
608+
State: v1.CapacityReservationStateActive,
609+
ReservationType: v1.CapacityReservationTypeDefault,
586610
},
587611
}
588612
for _, elem := range nodeClass.Status.CapacityReservations {

‎pkg/providers/instance/filter/filter.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,6 @@ func (f capacityReservationTypeFilter) Partition(instanceTypes []*cloudprovider.
138138
}
139139
}
140140
for _, it := range instanceTypes {
141-
142141
for _, o := range it.Offerings.Available().Compatible(f.requirements) {
143142
if o.CapacityType() != karpv1.CapacityTypeReserved {
144143
continue
@@ -203,7 +202,7 @@ func (f capacityBlockFilter) FilterReject(instanceTypes []*cloudprovider.Instanc
203202
}
204203

205204
func (f capacityBlockFilter) shouldFilter(instanceTypes []*cloudprovider.InstanceType) bool {
206-
if f.requirements.Get(karpv1.CapacityTypeLabelKey).Any() != karpv1.CapacityTypeReserved {
205+
if !f.requirements.Get(karpv1.CapacityTypeLabelKey).Has(karpv1.CapacityTypeReserved) {
207206
return false
208207
}
209208
for _, it := range instanceTypes {

‎pkg/providers/instance/filter/filter_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,60 @@ var _ = Describe("InstanceFiltersTest", func() {
259259
})
260260
})
261261

262+
Context("CapacityBlockFilter", func() {
263+
It("should select the instance type with the cheapest capacity-block offering", func() {
264+
f := filter.CapacityBlockFilter(scheduling.NewRequirements(scheduling.NewRequirement(karpv1.CapacityTypeLabelKey, corev1.NodeSelectorOpExists)))
265+
kept, rejected := f.FilterReject([]*cloudprovider.InstanceType{
266+
makeInstanceType("cheap-instance", withOfferings(
267+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(1.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
268+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(10.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
269+
)),
270+
makeInstanceType("expensive-instance", withOfferings(
271+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(2.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
272+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(10.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
273+
)),
274+
})
275+
expectInstanceTypes(kept, "cheap-instance")
276+
expectInstanceTypes(rejected, "expensive-instance")
277+
})
278+
DescribeTable(
279+
"shouldn't filter instance types when the capacity reservation type is not capacity-block",
280+
func(crt v1.CapacityReservationType) {
281+
f := filter.CapacityBlockFilter(scheduling.NewRequirements(scheduling.NewRequirement(karpv1.CapacityTypeLabelKey, corev1.NodeSelectorOpExists)))
282+
kept, rejected := f.FilterReject([]*cloudprovider.InstanceType{
283+
makeInstanceType("cheap-instance", withOfferings(
284+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(1.0), withCapacityReservationType(crt)),
285+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(10.0), withCapacityReservationType(crt)),
286+
)),
287+
makeInstanceType("expensive-instance", withOfferings(
288+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(2.0), withCapacityReservationType(crt)),
289+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(10.0), withCapacityReservationType(crt)),
290+
)),
291+
})
292+
expectInstanceTypes(kept, "cheap-instance", "expensive-instance")
293+
Expect(rejected).To(BeEmpty())
294+
},
295+
lo.FilterMap(v1.CapacityReservationType("").Values(), func(crt v1.CapacityReservationType, _ int) (TableEntry, bool) {
296+
return Entry(fmt.Sprintf("when the capacity reservation type is %q", string(crt)), crt), crt != v1.CapacityReservationTypeCapacityBlock
297+
}),
298+
)
299+
It("shouldn't filter instance types when the requirements aren't compatible with capacity type reserved", func() {
300+
f := filter.CapacityBlockFilter(scheduling.NewRequirements(scheduling.NewRequirement(karpv1.CapacityTypeLabelKey, corev1.NodeSelectorOpNotIn, karpv1.CapacityTypeReserved)))
301+
kept, rejected := f.FilterReject([]*cloudprovider.InstanceType{
302+
makeInstanceType("cheap-instance", withOfferings(
303+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(1.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
304+
makeOffering(karpv1.CapacityTypeOnDemand, true, withPrice(1.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
305+
)),
306+
makeInstanceType("expensive-instance", withOfferings(
307+
makeOffering(karpv1.CapacityTypeReserved, true, withPrice(2.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
308+
makeOffering(karpv1.CapacityTypeOnDemand, true, withPrice(2.0), withCapacityReservationType(v1.CapacityReservationTypeCapacityBlock)),
309+
)),
310+
})
311+
expectInstanceTypes(kept, "cheap-instance", "expensive-instance")
312+
Expect(rejected).To(BeEmpty())
313+
})
314+
})
315+
262316
Context("ReservedOfferingFilter", func() {
263317
var f filter.Filter
264318
BeforeEach(func() {

0 commit comments

Comments
 (0)