remaining e2es + feedback

pkg/controllers/nodeclaim/capacityreservation/controller.go

@@ -72,9 +72,6 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	if err := c.kubeClient.List(ctx, ncs); err != nil {
 		return reconcile.Result{}, fmt.Errorf("listing nodeclaims, %w", err)
 	}
-	log.FromContext(ctx).WithValues("provider-ids", lo.Keys(providerIDsToCPNodeClaims), "nodeclaims", lo.Map(ncs.Items, func(nc karpv1.NodeClaim, _ int) string {
-		return nc.Name
-	})).Info("evaluating")
 	updatedNodeClaims := sets.New[string]()
 	var errs []error
 	for i := range ncs.Items {

pkg/controllers/nodeclaim/capacityreservation/suite_test.go

@@ -154,16 +154,4 @@ var _ = Describe("Capacity Reservation NodeClaim Controller", func() {
 		Expect(node.Labels).To(HaveKeyWithValue(karpv1.CapacityTypeLabelKey, karpv1.CapacityTypeOnDemand))
 		Expect(node.Labels).ToNot(HaveKey(corecloudprovider.ReservationIDLabel))
 	})
-	It("should ignore nodeclaims which aren't registered", func() {
-		out := awsEnv.EC2API.DescribeInstancesBehavior.Output.Clone()
-		out.Reservations[0].Instances[0].CapacityReservationId = nil
-		awsEnv.EC2API.DescribeInstancesBehavior.Output.Set(out)
-		delete(nodeClaim.Labels, karpv1.NodeRegisteredLabelKey)
-
-		ExpectApplied(ctx, env.Client, nodeClaim)
-		ExpectSingletonReconciled(ctx, controller)
-		nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
-		Expect(nodeClaim.Labels).To(HaveKeyWithValue(karpv1.CapacityTypeLabelKey, karpv1.CapacityTypeReserved))
-		Expect(nodeClaim.Labels).To(HaveKeyWithValue(corecloudprovider.ReservationIDLabel, reservationID))
-	})
 })

pkg/providers/instancetype/offering/provider.go

@@ -106,6 +106,9 @@ func (p *DefaultProvider) InjectOfferings(
 			})
 		}
 
+		// NOTE: By making this copy one level deep, we can modify the offerings without mutating the results from previous
+		// GetInstanceTypes calls. This should still be done with caution - it is currently done here in the provider, and
+		// once in the instance provider (filterReservedInstanceTypes)
 		its = append(its, &cloudprovider.InstanceType{
 			Name:         it.Name,
 			Requirements: it.Requirements,

test/pkg/environment/aws/expectations.go

@@ -536,7 +536,7 @@ func ExpectCapacityReservationCreated(
 	capacity int32,
 	endDate *time.Time,
 	tags map[string]string,
-) (id string, cleanup func()) {
+) string {
 	GinkgoHelper()
 	out, err := ec2api.CreateCapacityReservation(ctx, &ec2.CreateCapacityReservationInput{
 		InstanceCount:         lo.ToPtr(capacity),
@@ -551,10 +551,14 @@ func ExpectCapacityReservationCreated(
 		}}, nil),
 	})
 	Expect(err).ToNot(HaveOccurred())
-	return lo.FromPtr(out.CapacityReservation.CapacityReservationId), func() {
-		GinkgoHelper()
+	return *out.CapacityReservation.CapacityReservationId
+}
+
+func ExpectCapacityReservationsCanceled(ctx context.Context, ec2api *ec2.Client, reservationIDs ...string) {
+	GinkgoHelper()
+	for _, id := range reservationIDs {
 		_, err := ec2api.CancelCapacityReservation(ctx, &ec2.CancelCapacityReservationInput{
-			CapacityReservationId: lo.ToPtr(*out.CapacityReservation.CapacityReservationId),
+			CapacityReservationId: &id,
 		})
 		Expect(err).ToNot(HaveOccurred())
 	}