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.

Sign up for GitHub

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

Make it possible to set node naming convention #1006

Merged
merged 1 commit into from
Dec 17, 2021
Merged

Make it possible to set node naming convention #1006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions pkg/cloudprovider/aws/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/aws/karpenter/pkg/apis/provisioning/v1alpha5"
"github.com/aws/karpenter/pkg/cloudprovider"
"github.com/aws/karpenter/pkg/cloudprovider/aws/apis/v1alpha1"
"github.com/aws/karpenter/pkg/utils/injection"
)

type InstanceProvider struct {
Expand Down Expand Up @@ -73,9 +74,8 @@ func (p *InstanceProvider) Create(ctx context.Context, constraints *v1alpha1.Con
aws.StringValue(instance.Placement.AvailabilityZone),
getCapacityType(instance),
)

// Convert Instance to Node
node, err := p.instanceToNode(instance, instanceTypes)
node, err := p.instanceToNode(ctx, instance, instanceTypes)
if err != nil {
logging.FromContext(ctx).Errorf("creating Node from an EC2 Instance: %s", err.Error())
continue
Expand Down Expand Up @@ -229,12 +229,16 @@ func (p *InstanceProvider) getInstances(ctx context.Context, ids []*string) ([]*
return instances, err
}

func (p *InstanceProvider) instanceToNode(instance *ec2.Instance, instanceTypes []cloudprovider.InstanceType) (*v1.Node, error) {
func (p *InstanceProvider) instanceToNode(ctx context.Context, instance *ec2.Instance, instanceTypes []cloudprovider.InstanceType) (*v1.Node, error) {
for _, instanceType := range instanceTypes {
if instanceType.Name() == aws.StringValue(instance.InstanceType) {
nodeName := strings.ToLower(aws.StringValue(instance.PrivateDnsName))
if injection.GetOptions(ctx).AWSNodeNameConvention == "resource-name" {
nodeName = aws.StringValue(instance.InstanceId)
}
return &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: aws.StringValue(instance.PrivateDnsName),
Name: nodeName,
Labels: map[string]string{
v1.LabelTopologyZone: aws.StringValue(instance.Placement.AvailabilityZone),
v1.LabelInstanceTypeStable: aws.StringValue(instance.InstanceType),
Expand Down
8 changes: 7 additions & 1 deletion pkg/cloudprovider/aws/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@ func TestAPIs(t *testing.T) {

var _ = BeforeSuite(func() {
env = test.NewEnvironment(ctx, func(e *test.Environment) {
ctx = injection.WithOptions(ctx, options.Options{ClusterName: "test-cluster", ClusterEndpoint: "https://test-cluster"})
opts := options.Options{
ClusterName: "test-cluster",
ClusterEndpoint: "https://test-cluster",
AWSNodeNameConvention: "ip-name",
}
Expect(opts.Validate()).To(Succeed(), "Failed to validate options")
ctx = injection.WithOptions(ctx, opts)
launchTemplateCache = cache.New(CacheTTL, CacheCleanupInterval)
unavailableOfferingsCache = cache.New(InsufficientCapacityErrorCacheTTL, InsufficientCapacityErrorCacheCleanupInterval)
fakeEC2API = &fake.EC2API{}
Expand Down
19 changes: 12 additions & 7 deletions pkg/utils/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func MustParse() Options {
flag.IntVar(&opts.WebhookPort, "port", 8443, "The port the webhook endpoint binds to for validation and mutation of resources")
flag.IntVar(&opts.KubeClientQPS, "kube-client-qps", env.WithDefaultInt("KUBE_CLIENT_QPS", 200), "The smoothed rate of qps to kube-apiserver")
flag.IntVar(&opts.KubeClientBurst, "kube-client-burst", env.WithDefaultInt("KUBE_CLIENT_BURST", 300), "The maximum allowed burst of queries to the kube-apiserver")
flag.StringVar(&opts.AWSNodeNameConvention, "aws-node-name-convention", env.WithDefaultString("AWS_NODE_NAME_CONVENTION", "ip-name"), "The node naming convention used by the AWS cloud provider. DEPRECATION WARNING: this field may be deprecated at any time")
flag.Parse()
if err := opts.Validate(); err != nil {
panic(err)
Expand All @@ -41,20 +42,24 @@ func MustParse() Options {

// Options for running this binary
type Options struct {
ClusterName string
ClusterEndpoint string
MetricsPort int
HealthProbePort int
WebhookPort int
KubeClientQPS int
KubeClientBurst int
ClusterName string
ClusterEndpoint string
MetricsPort int
HealthProbePort int
WebhookPort int
KubeClientQPS int
KubeClientBurst int
AWSNodeNameConvention string
}

func (o Options) Validate() (err error) {
err = multierr.Append(err, o.validateEndpoint())
if o.ClusterName == "" {
err = multierr.Append(err, fmt.Errorf("CLUSTER_NAME is required"))
}
if o.AWSNodeNameConvention != "ip-name" && o.AWSNodeNameConvention != "resource-name" {
Copy link
Contributor

@ellistarn ellistarn Dec 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we default to ip-name, for now? I'd hate for folks to have to pass this flag in all cases.

edit: nvm, you did this in the flag above. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ip-name is the default when parsing the flags. There shouldn't be a need to set this explicitly when running through main(). At least that was my intention.

err = multierr.Append(err, fmt.Errorf("aws-node-name-convention may only be either ip-name or resource-name"))
}
return err
}

Expand Down