Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add imageID status field #4637

Merged
merged 60 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
6243188
feat: add eu-central-1 as pricing endpoint for eu-* regions
gfcroft Aug 21, 2023
d012f7c
Merge branch 'main' of github.com:aws/karpenter
gfcroft Aug 28, 2023
e20607c
Merge branch 'main' of github.com:aws/karpenter
gfcroft Sep 10, 2023
ac40626
feat: Add karpenter.k8s.aws/instance-ami-id label
gfcroft Sep 10, 2023
11cf6fb
feat: update necessary permissions to allow karpenter controller to l…
gfcroft Sep 10, 2023
f040694
fix lint issues
gfcroft Sep 12, 2023
034cbb3
fix tests
gfcroft Sep 15, 2023
f64363c
do codegen. Upgrade vulnerable packages
gfcroft Sep 15, 2023
b7771a8
remove abandoned change to a mock test function
gfcroft Sep 15, 2023
5988dd9
remove unintended newline
gfcroft Sep 15, 2023
ffa16fb
remove wip local dep TODO comment
gfcroft Sep 18, 2023
618dfc8
remove attempt to resolve ami-id label in case where custom launch te…
gfcroft Sep 19, 2023
c88adad
remove describeTemplateVersions related additions from fake ec2api no…
gfcroft Sep 19, 2023
7e853a7
Revert "feat: update necessary permissions to allow karpenter control…
gfcroft Sep 19, 2023
4adf91e
remove codegen content from this pr and resolve nit. issues
gfcroft Sep 21, 2023
ff5d139
Merge branch 'main' of github.com:aws/karpenter
gfcroft Sep 21, 2023
1bc2200
lint fix
gfcroft Sep 22, 2023
0365a70
Merge branch 'main' of github.com:aws/karpenter
gfcroft Sep 24, 2023
8b9d3a0
fix failing tests after merge
gfcroft Sep 24, 2023
1d259fa
remove mistakenly included tests which should have been removed after…
gfcroft Sep 24, 2023
490998d
Update pkg/cloudprovider/cloudprovider.go
gfcroft Sep 25, 2023
273c9d5
Update pkg/providers/launchtemplate/launchtemplate.go
gfcroft Sep 25, 2023
7d25f2f
Merge branch 'main' of github.com:aws/karpenter
gfcroft Sep 25, 2023
1ae5349
Merge branch 'main' of github.com:/gfcroft/karpenter
gfcroft Sep 25, 2023
2ffd71a
add LabelInstanceAMIID to v1beta1 api pkg
gfcroft Sep 25, 2023
4a7b932
fix syntax syntax error
gfcroft Sep 25, 2023
9c524fe
feat: Add karpenter.k8s.aws/instance-ami-id label
gfcroft Sep 10, 2023
d8de375
feat: update necessary permissions to allow karpenter controller to l…
gfcroft Sep 10, 2023
625643f
fix lint issues
gfcroft Sep 12, 2023
57a2ab1
fix tests
gfcroft Sep 15, 2023
3449cf9
do codegen. Upgrade vulnerable packages
gfcroft Sep 15, 2023
551a114
remove abandoned change to a mock test function
gfcroft Sep 15, 2023
1dc3a7e
remove unintended newline
gfcroft Sep 15, 2023
6ee56a6
remove wip local dep TODO comment
gfcroft Sep 18, 2023
5025ddb
remove attempt to resolve ami-id label in case where custom launch te…
gfcroft Sep 19, 2023
53b29af
remove describeTemplateVersions related additions from fake ec2api no…
gfcroft Sep 19, 2023
3a01052
Revert "feat: update necessary permissions to allow karpenter control…
gfcroft Sep 19, 2023
22dff06
remove codegen content from this pr and resolve nit. issues
gfcroft Sep 21, 2023
21b71a6
lint fix
gfcroft Sep 22, 2023
c345730
fix failing tests after merge
gfcroft Sep 24, 2023
e457128
Update pkg/cloudprovider/cloudprovider.go
gfcroft Sep 25, 2023
3f29371
Update pkg/providers/launchtemplate/launchtemplate.go
gfcroft Sep 25, 2023
97fb245
add LabelInstanceAMIID to v1beta1 api pkg
gfcroft Sep 25, 2023
fb3510d
fix syntax syntax error
gfcroft Sep 25, 2023
ff8afd3
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 1, 2023
fe66c29
add imageId to nodeClaim status field. Depends on addition of ImageId…
gfcroft Oct 1, 2023
b76960b
Merge branch 'main' of github.com:/gfcroft/karpenter
gfcroft Oct 1, 2023
44e35d6
update go.mod,go.sum and api/crds following karpenter-core change to …
gfcroft Oct 2, 2023
8fe4b5e
add unit test to check that Status.ImageID field is populated on node…
gfcroft Oct 2, 2023
c01aec0
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 3, 2023
1cb1838
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 3, 2023
14f4946
Merge branch 'main' into main
gfcroft Oct 5, 2023
b1c05e6
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 5, 2023
67f055d
remove redundant label addition to v1beta1 labels and generate file c…
gfcroft Oct 5, 2023
9bdb097
Merge branch 'main' of github.com:/gfcroft/karpenter
gfcroft Oct 5, 2023
87e45c9
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 10, 2023
0110690
Merge branch 'main' into main
gfcroft Oct 10, 2023
0f06c25
Merge branch 'main' of github.com:aws/karpenter
gfcroft Oct 10, 2023
a533c4b
Merge branch 'main' of github.com:/gfcroft/karpenter
gfcroft Oct 10, 2023
535d34b
fix - use the right overrides for each fleet launch template request
gfcroft Oct 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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