Skip to content

Commit

Permalink
feat: Add imageID status field (#4637)
Browse files Browse the repository at this point in the history
Co-authored-by: Jonathan Innis <jonathan.innis.ji@gmail.com>
  • Loading branch information
gfcroft and jonathan-innis committed Oct 11, 2023
1 parent a45c1fe commit 33a3a7f
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 15 deletions.
1 change: 1 addition & 0 deletions pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,5 +379,6 @@ func (c *CloudProvider) instanceToNodeClaim(i *instance.Instance, instanceType *
nodeClaim.DeletionTimestamp = &metav1.Time{Time: time.Now()}
}
nodeClaim.Status.ProviderID = fmt.Sprintf("aws:///%s/%s", i.Zone, i.ID)
nodeClaim.Status.ImageID = i.ImageID
return nodeClaim
}
6 changes: 3 additions & 3 deletions pkg/cloudprovider/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,9 @@ var _ = Describe("Machine/CloudProvider", func() {
if *ov.InstanceType == "m5.large" {
foundNonGPULT = true
Expect(v.Overrides).To(ContainElements(
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test1"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1a")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test2"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1b")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test3"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1c")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test1"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1a")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test2"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1b")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test3"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1c")},
))
}
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/cloudprovider/nodeclaim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
Expect(corecloudproivder.IsInsufficientCapacityError(err)).To(BeTrue())
Expect(cloudProviderNodeClaim).To(BeNil())
})
It("should set ImageID in the status field of the nodeClaim", func() {
ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim)
cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim)
Expect(err).To(BeNil())
Expect(cloudProviderNodeClaim).ToNot(BeNil())
Expect(cloudProviderNodeClaim.Status.ImageID).ToNot(BeEmpty())
})
It("should return NodeClass Hash on the nodeClaim", func() {
ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim)
cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim)
Expand Down Expand Up @@ -373,9 +380,9 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
if *ov.InstanceType == "m5.large" {
foundNonGPULT = true
Expect(v.Overrides).To(ContainElements(
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test1"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1a")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test2"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1b")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test3"), InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1c")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test1"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1a")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test2"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1b")},
&ec2.FleetLaunchTemplateOverridesRequest{SubnetId: aws.String("subnet-test3"), ImageId: ov.ImageId, InstanceType: aws.String("m5.large"), AvailabilityZone: aws.String("test-zone-1c")},
))
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/fake/ec2api.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func (e *EC2API) CreateFleetWithContext(_ context.Context, input *ec2.CreateFlee
LaunchTemplateAndOverrides: &ec2.LaunchTemplateAndOverridesResponse{
Overrides: &ec2.FleetLaunchTemplateOverrides{
SubnetId: input.LaunchTemplateConfigs[0].Overrides[0].SubnetId,
ImageId: input.LaunchTemplateConfigs[0].Overrides[0].ImageId,
InstanceType: input.LaunchTemplateConfigs[0].Overrides[0].InstanceType,
AvailabilityZone: input.LaunchTemplateConfigs[0].Overrides[0].AvailabilityZone,
},
Expand Down
9 changes: 5 additions & 4 deletions pkg/providers/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,11 @@ func (p *Provider) getLaunchTemplateConfigs(ctx context.Context, nodeClass *v1be
if err != nil {
return nil, fmt.Errorf("getting launch templates, %w", err)
}
for launchTemplateName, instanceTypes := range launchTemplates {
for _, launchTemplate := range launchTemplates {
launchTemplateConfig := &ec2.FleetLaunchTemplateConfigRequest{
Overrides: p.getOverrides(instanceTypes, zonalSubnets, scheduling.NewNodeSelectorRequirements(nodeClaim.Spec.Requirements...).Get(v1.LabelTopologyZone), capacityType),
Overrides: p.getOverrides(launchTemplate.InstanceTypes, zonalSubnets, scheduling.NewNodeSelectorRequirements(nodeClaim.Spec.Requirements...).Get(v1.LabelTopologyZone), capacityType, launchTemplate.ImageID),
LaunchTemplateSpecification: &ec2.FleetLaunchTemplateSpecificationRequest{
LaunchTemplateName: aws.String(launchTemplateName),
LaunchTemplateName: aws.String(launchTemplate.Name),
Version: aws.String("$Latest"),
},
}
Expand All @@ -321,7 +321,7 @@ func (p *Provider) getLaunchTemplateConfigs(ctx context.Context, nodeClass *v1be

// getOverrides creates and returns launch template overrides for the cross product of InstanceTypes and subnets (with subnets being constrained by
// zones and the offerings in InstanceTypes)
func (p *Provider) getOverrides(instanceTypes []*cloudprovider.InstanceType, zonalSubnets map[string]*ec2.Subnet, zones *scheduling.Requirement, capacityType string) []*ec2.FleetLaunchTemplateOverridesRequest {
func (p *Provider) getOverrides(instanceTypes []*cloudprovider.InstanceType, zonalSubnets map[string]*ec2.Subnet, zones *scheduling.Requirement, capacityType string, image string) []*ec2.FleetLaunchTemplateOverridesRequest {
// Unwrap all the offerings to a flat slice that includes a pointer
// to the parent instance type name
type offeringWithParentName struct {
Expand Down Expand Up @@ -354,6 +354,7 @@ func (p *Provider) getOverrides(instanceTypes []*cloudprovider.InstanceType, zon
overrides = append(overrides, &ec2.FleetLaunchTemplateOverridesRequest{
InstanceType: aws.String(offering.parentInstanceTypeName),
SubnetId: subnet.SubnetId,
ImageId: aws.String(image),
// This is technically redundant, but is useful if we have to parse insufficient capacity errors from
// CreateFleet so that we can figure out the zone rather than additional API calls to look up the subnet
AvailabilityZone: subnet.AvailabilityZone,
Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/instance/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func NewInstanceFromFleet(out *ec2.CreateFleetInstance, tags map[string]string)
LaunchTime: time.Now(), // estimate the launch time since we just launched
State: ec2.StatePending,
ID: aws.StringValue(out.InstanceIds[0]),
ImageID: "", // we don't know the image id when we get the output from fleet
ImageID: aws.StringValue(out.LaunchTemplateAndOverrides.Overrides.ImageId),
Type: aws.StringValue(out.InstanceType),
Zone: aws.StringValue(out.LaunchTemplateAndOverrides.Overrides.AvailabilityZone),
CapacityType: aws.StringValue(out.Lifecycle),
Expand Down
15 changes: 11 additions & 4 deletions pkg/providers/launchtemplate/launchtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ const (
karpenterManagedTagKey = "karpenter.k8s.aws/cluster"
)

type LaunchTemplate struct {
Name string
InstanceTypes []*cloudprovider.InstanceType
ImageID string
}

type Provider struct {
sync.Mutex
ec2api ec2iface.EC2API
Expand Down Expand Up @@ -97,14 +103,15 @@ func NewProvider(ctx context.Context, cache *cache.Cache, ec2api ec2iface.EC2API
}

func (p *Provider) EnsureAll(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, nodeClaim *corev1beta1.NodeClaim,
instanceTypes []*cloudprovider.InstanceType, additionalLabels map[string]string, tags map[string]string) (map[string][]*cloudprovider.InstanceType, error) {
instanceTypes []*cloudprovider.InstanceType, additionalLabels map[string]string, tags map[string]string) ([]*LaunchTemplate, error) {

p.Lock()
defer p.Unlock()
// If Launch Template is directly specified then just use it
if nodeClass.Spec.LaunchTemplateName != nil {
return map[string][]*cloudprovider.InstanceType{ptr.StringValue(nodeClass.Spec.LaunchTemplateName): instanceTypes}, nil
return []*LaunchTemplate{{Name: ptr.StringValue(nodeClass.Spec.LaunchTemplateName), InstanceTypes: instanceTypes}}, nil
}

options, err := p.createAMIOptions(ctx, nodeClass, lo.Assign(nodeClaim.Labels, additionalLabels), tags)
if err != nil {
return nil, err
Expand All @@ -113,14 +120,14 @@ func (p *Provider) EnsureAll(ctx context.Context, nodeClass *v1beta1.EC2NodeClas
if err != nil {
return nil, err
}
launchTemplates := map[string][]*cloudprovider.InstanceType{}
var launchTemplates []*LaunchTemplate
for _, resolvedLaunchTemplate := range resolvedLaunchTemplates {
// Ensure the launch template exists, or create it
ec2LaunchTemplate, err := p.ensureLaunchTemplate(ctx, resolvedLaunchTemplate)
if err != nil {
return nil, err
}
launchTemplates[*ec2LaunchTemplate.LaunchTemplateName] = resolvedLaunchTemplate.InstanceTypes
launchTemplates = append(launchTemplates, &LaunchTemplate{Name: *ec2LaunchTemplate.LaunchTemplateName, InstanceTypes: resolvedLaunchTemplate.InstanceTypes, ImageID: resolvedLaunchTemplate.AMIID})
}
return launchTemplates, nil
}
Expand Down

0 comments on commit 33a3a7f

Please sign in to comment.