diff --git a/pkg/providers/instancetype/instancetype.go b/pkg/providers/instancetype/instancetype.go index a40111129ddc..82bbcb099faa 100644 --- a/pkg/providers/instancetype/instancetype.go +++ b/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 { diff --git a/pkg/providers/subnet/subnet.go b/pkg/providers/subnet/subnet.go index 3cd3d3a7e88e..15fe1c84bd3e 100644 --- a/pkg/providers/subnet/subnet.go +++ b/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 } diff --git a/test/suites/integration/scheduling_test.go b/test/suites/integration/scheduling_test.go index d755a65e731d..be328ed5cdd2 100644 --- a/test/suites/integration/scheduling_test.go +++ b/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,