Skip to content

Commit

Permalink
middle of fixing unit tests, TODO: write e2etests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rschalo
rschalo committed May 22, 2024
1 parent dd2cc3c commit 0f3a89d
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 103 deletions.
2 changes: 0 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,6 @@ sigs.k8s.io/controller-runtime v0.18.2 h1:RqVW6Kpeaji67CY5nPEfRz6ZfFMk0lWQlNrLql
sigs.k8s.io/controller-runtime v0.18.2/go.mod h1:tuAt1+wbVsXIT8lPtk5RURxqAnq7xkpv2Mhttslg7Hw=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/karpenter v0.36.1-0.20240521002315-9b145a6d85b4 h1:zIKW8TX593mp/rlOdCqIbgUdVRQGHzeFkgDM6+zgeE8=
sigs.k8s.io/karpenter v0.36.1-0.20240521002315-9b145a6d85b4/go.mod h1:5XYrIz9Bi7HgQyaUsx7O08ft+TJjrH+htlnPq8Sz9J8=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
Expand Down
131 changes: 78 additions & 53 deletions pkg/controllers/nodeclass/status/subnet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,44 +55,51 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
{
ID: "subnet-test3",
Zone: "test-zone-1c",
ID: "subnet-test3",
Zone: "test-zone-1c",
ZoneID: "tstz1-1c",
},
{
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ZoneID: "tstz1-1alocal",
},
}))
})
It("Should have the correct ordering for the Subnets", func() {
awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{Subnets: []*ec2.Subnet{
{SubnetId: aws.String("subnet-test1"), AvailabilityZone: aws.String("test-zone-1a"), AvailableIpAddressCount: aws.Int64(20)},
{SubnetId: aws.String("subnet-test2"), AvailabilityZone: aws.String("test-zone-1b"), AvailableIpAddressCount: aws.Int64(100)},
{SubnetId: aws.String("subnet-test3"), AvailabilityZone: aws.String("test-zone-1c"), AvailableIpAddressCount: aws.Int64(50)},
{SubnetId: aws.String("subnet-test1"), AvailabilityZone: aws.String("test-zone-1a"), AvailabilityZoneId: aws.String("tstz1-1a"), AvailableIpAddressCount: aws.Int64(20)},
{SubnetId: aws.String("subnet-test2"), AvailabilityZone: aws.String("test-zone-1b"), AvailabilityZoneId: aws.String("tstz1-1b"), AvailableIpAddressCount: aws.Int64(100)},
{SubnetId: aws.String("subnet-test3"), AvailabilityZone: aws.String("test-zone-1c"), AvailabilityZoneId: aws.String("tstz1-1c"), AvailableIpAddressCount: aws.Int64(50)},
}})
ExpectApplied(ctx, env.Client, nodeClass)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
{
ID: "subnet-test3",
Zone: "test-zone-1c",
ID: "subnet-test3",
Zone: "test-zone-1c",
ZoneID: "tstz1-1c",
},
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
}))
})
Expand All @@ -110,12 +117,14 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
}))
})
Expand All @@ -130,8 +139,9 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
}))
})
Expand All @@ -141,20 +151,24 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
{
ID: "subnet-test3",
Zone: "test-zone-1c",
ID: "subnet-test3",
Zone: "test-zone-1c",
ZoneID: "tstz1-1c",
},
{
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ZoneID: "tstz1-1alocal",
},
}))

Expand All @@ -175,12 +189,14 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
}))
})
Expand All @@ -190,20 +206,24 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
{
ID: "subnet-test3",
Zone: "test-zone-1c",
ID: "subnet-test3",
Zone: "test-zone-1c",
ZoneID: "tstz1-1c",
},
{
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ZoneID: "tstz1-1alocal",
},
}))

Expand All @@ -217,8 +237,9 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
}))
})
Expand All @@ -241,20 +262,24 @@ var _ = Describe("NodeClass Subnet Status Controller", func() {
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{
{
ID: "subnet-test1",
Zone: "test-zone-1a",
ID: "subnet-test1",
Zone: "test-zone-1a",
ZoneID: "tstz1-1a",
},
{
ID: "subnet-test2",
Zone: "test-zone-1b",
ID: "subnet-test2",
Zone: "test-zone-1b",
ZoneID: "tstz1-1b",
},
{
ID: "subnet-test3",
Zone: "test-zone-1c",
ID: "subnet-test3",
Zone: "test-zone-1c",
ZoneID: "tstz1-1c",
},
{
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ID: "subnet-test4",
Zone: "test-zone-1a-local",
ZoneID: "tstz1-1alocal",
},
}))

Expand Down
3 changes: 1 addition & 2 deletions pkg/controllers/providers/instancetype/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
coreoptions "sigs.k8s.io/karpenter/pkg/operator/options"
Expand Down Expand Up @@ -151,7 +150,7 @@ var _ = Describe("InstanceType", func() {
})
Expect(found).To(BeTrue())
for y := range instanceTypes[x].Offerings {
Expect(instanceTypes[x].Offerings[y].Constraints[v1.LabelTopologyZone]).To(Equal(lo.FromPtr(offering.Location)))
Expect(instanceTypes[x].Offerings[y].Zone()).To(Equal(lo.FromPtr(offering.Location)))
}
}
})
Expand Down
4 changes: 4 additions & 0 deletions pkg/fake/ec2api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ func (e *EC2API) DescribeSubnetsWithContext(_ context.Context, input *ec2.Descri
{
SubnetId: aws.String("subnet-test1"),
AvailabilityZone: aws.String("test-zone-1a"),
AvailabilityZoneId: aws.String("tstz1-1a"),
AvailableIpAddressCount: aws.Int64(100),
MapPublicIpOnLaunch: aws.Bool(false),
Tags: []*ec2.Tag{
Expand All @@ -428,6 +429,7 @@ func (e *EC2API) DescribeSubnetsWithContext(_ context.Context, input *ec2.Descri
{
SubnetId: aws.String("subnet-test2"),
AvailabilityZone: aws.String("test-zone-1b"),
AvailabilityZoneId: aws.String("tstz1-1b"),
AvailableIpAddressCount: aws.Int64(100),
MapPublicIpOnLaunch: aws.Bool(true),
Tags: []*ec2.Tag{
Expand All @@ -438,6 +440,7 @@ func (e *EC2API) DescribeSubnetsWithContext(_ context.Context, input *ec2.Descri
{
SubnetId: aws.String("subnet-test3"),
AvailabilityZone: aws.String("test-zone-1c"),
AvailabilityZoneId: aws.String("tstz1-1c"),
AvailableIpAddressCount: aws.Int64(100),
Tags: []*ec2.Tag{
{Key: aws.String("Name"), Value: aws.String("test-subnet-3")},
Expand All @@ -448,6 +451,7 @@ func (e *EC2API) DescribeSubnetsWithContext(_ context.Context, input *ec2.Descri
{
SubnetId: aws.String("subnet-test4"),
AvailabilityZone: aws.String("test-zone-1a-local"),
AvailabilityZoneId: aws.String("tstz1-1alocal"),
AvailableIpAddressCount: aws.Int64(100),
MapPublicIpOnLaunch: aws.Bool(true),
Tags: []*ec2.Tag{
Expand Down
16 changes: 8 additions & 8 deletions pkg/providers/instance/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,14 @@ func (p *DefaultProvider) getOverrides(instanceTypes []*cloudprovider.InstanceTy
}
var overrides []*ec2.FleetLaunchTemplateOverridesRequest
for _, offering := range unwrappedOfferings {
if capacityType != offering.Constraints[corev1beta1.CapacityTypeLabelKey] {
if capacityType != offering.CapacityType() {
continue
}
if !reqs.Get(v1.LabelTopologyZone).Has(offering.Constraints[v1.LabelTopologyZone]) ||
!reqs.Get(v1beta1.LabelInstanceAvailabilityZoneID).Has(offering.Constraints[v1beta1.LabelInstanceAvailabilityZoneID]) {
if !reqs.Get(v1.LabelTopologyZone).Has(offering.Zone()) ||
!reqs.Get(v1beta1.LabelInstanceAvailabilityZoneID).Has(offering.CapacityType()) {
continue
}
subnet, ok := zonalSubnets[offering.Constraints[v1.LabelTopologyZone]]
subnet, ok := zonalSubnets[offering.Zone()]
if !ok {
continue
}
Expand Down Expand Up @@ -378,7 +378,7 @@ func (p *DefaultProvider) getCapacityType(nodeClaim *corev1beta1.NodeClaim, inst
if requirements.Get(corev1beta1.CapacityTypeLabelKey).Has(corev1beta1.CapacityTypeSpot) {
for _, instanceType := range instanceTypes {
for _, offering := range instanceType.Offerings.Available() {
if requirements.Get(v1.LabelTopologyZone).Has(offering.Constraints[v1.LabelTopologyZone]) && offering.Constraints[corev1beta1.CapacityTypeLabelKey] == corev1beta1.CapacityTypeSpot {
if requirements.Get(v1.LabelTopologyZone).Has(offering.Zone()) && offering.CapacityType() == corev1beta1.CapacityTypeSpot {
return corev1beta1.CapacityTypeSpot
}
}
Expand Down Expand Up @@ -413,8 +413,8 @@ func (p *DefaultProvider) isMixedCapacityLaunch(nodeClaim *corev1beta1.NodeClaim
if requirements.Get(corev1beta1.CapacityTypeLabelKey).Has(corev1beta1.CapacityTypeSpot) {
for _, instanceType := range instanceTypes {
for _, offering := range instanceType.Offerings.Available() {
if requirements.Get(v1.LabelTopologyZone).Has(offering.Constraints[v1.LabelTopologyZone]) {
if offering.Constraints[corev1beta1.CapacityTypeLabelKey] == corev1beta1.CapacityTypeSpot {
if requirements.Get(v1.LabelTopologyZone).Has(offering.Zone()) {
if offering.CapacityType() == corev1beta1.CapacityTypeSpot {
hasSpotOfferings = true
} else {
hasODOffering = true
Expand All @@ -433,7 +433,7 @@ func filterUnwantedSpot(instanceTypes []*cloudprovider.InstanceType) []*cloudpro
// first, find the price of our cheapest available on-demand instance type that could support this node
for _, it := range instanceTypes {
for _, o := range it.Offerings.Available() {
if o.Constraints[corev1beta1.CapacityTypeLabelKey] == corev1beta1.CapacityTypeOnDemand && o.Price < cheapestOnDemand {
if o.CapacityType() == corev1beta1.CapacityTypeOnDemand && o.Price < cheapestOnDemand {
cheapestOnDemand = o.Price
}
}
Expand Down
20 changes: 6 additions & 14 deletions pkg/providers/instancetype/instancetype.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"

corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
"sigs.k8s.io/karpenter/pkg/scheduling"

"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
awscache "github.com/aws/karpenter-provider-aws/pkg/cache"
Expand Down Expand Up @@ -274,13 +275,12 @@ func (p *DefaultProvider) createOfferings(ctx context.Context, instanceType *ec2
}
available := !isUnavailable && ok && instanceTypeZones.Has(zone) && subnetZones.Has(zone)

// supported on 1.30 and not less
offerings = append(offerings, cloudprovider.Offering{
Constraints: map[string]string{
corev1beta1.CapacityTypeLabelKey: capacityType,
v1.LabelTopologyZone: zone,
v1beta1.LabelInstanceAvailabilityZoneID: zoneInfo[zone],
},
Requirements: scheduling.NewRequirements(
scheduling.NewRequirement(corev1beta1.CapacityTypeLabelKey, v1.NodeSelectorOpIn, capacityType),
scheduling.NewRequirement(v1.LabelTopologyZone, v1.NodeSelectorOpIn, zone),
scheduling.NewRequirement(v1beta1.LabelInstanceAvailabilityZoneID, v1.NodeSelectorOpIn, zoneInfo[zone]),
),
Price: price,
Available: available,
})
Expand All @@ -304,11 +304,3 @@ func (p *DefaultProvider) Reset() {
p.instanceTypeOfferings = map[string]sets.Set[string]{}
p.instanceTypesCache.Flush()
}

// func requirements(ct, zone, zoneID string) scheduling.Requirements {
// return scheduling.NewLabelRequirements(map[string]string{
// corev1beta1.CapacityTypeLabelKey: ct,
// v1.LabelTopologyZone: zone,
// v1beta1.LabelInstanceAvailabilityZoneID: zoneID,
// })
// }
Loading

0 comments on commit 0f3a89d

Please sign in to comment.