remaining comments

pkg/providers/instancetype/instancetype.go

@@ -252,6 +252,15 @@ func (p *DefaultProvider) UpdateInstanceTypeOfferings(ctx context.Context) error
 	return nil
 }
 
+// createOfferings creates a set of mutually exclusive offerings for a given instance type. This provider maintains an
+// invariant that each offering is mutually exclusive. Specifically, there is an offering for each permutation of zone
+// and capacity type. ZoneID is also injected into the offering requirements, when available, but there is a 1-1
+// mapping between zone and zoneID so this does not change the number of offerings.
+//
+// Each requirement on the offering is guaranteed to have a single value. To get the value for a requirement on an
+// offering, you can do the following thanks to this invariant:
+//
+//	offering.Requirements.Get(v1.TopologyLabelZone).Any()
 func (p *DefaultProvider) createOfferings(ctx context.Context, instanceType *ec2.InstanceTypeInfo, zones, instanceTypeZones sets.Set[string], subnets []v1beta1.Subnet) []cloudprovider.Offering {
 	var offerings []cloudprovider.Offering
 	for zone := range zones {

pkg/providers/subnet/subnet.go

@@ -112,10 +112,17 @@ func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
 	p.cache.SetDefault(fmt.Sprint(hash), lo.Values(subnets))
 	if p.cm.HasChanged(fmt.Sprintf("subnets/%s", nodeClass.Name), subnets) {
 		log.FromContext(ctx).
-			WithValues("subnets", lo.Map(lo.Values(subnets), func(s *ec2.Subnet, _ int) string {
-				return fmt.Sprintf(`"subnet": {"id": %q, "zone": %q, "zoneID": %q}`, aws.StringValue(s.SubnetId), aws.StringValue(s.AvailabilityZone), aws.StringValue(s.AvailabilityZoneId))
-			})).
-			V(1).Info("discovered subnets")
+			WithValues("subnets", lo.Map(lo.Values(subnets), func(s *ec2.Subnet, _ int) interface{} {
+				return struct {
+					ID     string `json:"id"`
+					Zone   string `json:"zone"`
+					ZoneID string `json:"zoneID,omitempty"`
+				}{
+					ID:     lo.FromPtr(s.SubnetId),
+					Zone:   lo.FromPtr(s.AvailabilityZone),
+					ZoneID: lo.FromPtr(s.AvailabilityZoneId),
+				}
+			})).V(1).Info("discovered subnets")
 	}
 	return lo.Values(subnets), nil
 }

test/suites/integration/scheduling_test.go

@@ -624,7 +624,12 @@ var _ = Describe("Scheduling", Ordered, ContinueOnFailure, func() {
 			Expect(node.Labels[v1beta1.LabelTopologyZoneID]).To(Equal(subnetInfo[1].ZoneID))
 		})
 		It("should provision nodes for pods with zone-id requirements in the correct zone", func() {
-			const expectedZoneLabel = "domain-label"
+			// Each pod specifies a requirement on this expected zone, where the value is the matching zone for the
+			// required zone-id. This allows us to verify that Karpenter launched the node in the correct zone, even if
+			// it doesn't add the zone-id label and the label is added by CCM. If we didn't take this approach, we would
+			// succeed even if Karpenter doesn't add the label and /or incorrectly generated offerings on k8s 1.30 and
+			// above. This is an unlikely scenario, and adding this check is a defense in depth measure.
+			const expectedZoneLabel = "expected-zone-label"
 			test.ReplaceRequirements(nodePool, corev1beta1.NodeSelectorRequirementWithMinValues{
 				NodeSelectorRequirement: v1.NodeSelectorRequirement{
 					Key:      expectedZoneLabel,