Use PrivateDnsName as Node name in nodeadm #1715
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cartermckinnon
force-pushed
the
nodeadm-privatednsname
branch
3 times, most recently
from
0657a2b
to
28671c8
Compare
cartermckinnon
commented
Mar 8, 2024
|
|
||
| // Discovers all cluster details using a describe call to the eks endpoint and | ||
| // updates the value of the config's `ClusterDetails` in-place | ||
| func populateClusterDetails(eksClient *eks.EKS, clusterName string, cfg *api.NodeConfig) error { |
There was a problem hiding this comment.
This was unused and we've decided to require these args in the user data.
cartermckinnon
force-pushed
the
nodeadm-privatednsname
branch
2 times, most recently
from
a7014d6
to
0d404a2
Compare
ndbaker1
reviewed
Mar 9, 2024
|
|
||
| "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" | ||
| "github.com/aws/aws-sdk-go/service/eks" | ||
| "github.com/aws/aws-sdk-go-v2/service/ec2" |
There was a problem hiding this comment.
It's the least I could do
ndbaker1
reviewed
Mar 9, 2024
ndbaker1
reviewed
Mar 9, 2024
| @@ -249,7 +249,9 @@ func (ksc *kubeletConfig) withCloudProvider(kubeletVersion string, cfg *api.Node | |||
| flags["cloud-provider"] = "external" | |||
| // provider ID needs to be specified when the cloud provider is external | |||
| ksc.ProviderID = ptr.String(getProviderId(cfg.Status.Instance.AvailabilityZone, cfg.Status.Instance.ID)) | |||
| // TODO set the --hostname-override equal to the EC2 PrivateDnsName | |||
There was a problem hiding this comment.
IIRC think there's a unit test with the corresponding TODO
cartermckinnon
commented
Mar 9, 2024
There was a problem hiding this comment.
this is just a fork of the internal waiter impls in the SDK, very little net-new code
|
/ci Seeing what the runtime behavior looks like while I update the e2e tests... |
|
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
|
@cartermckinnon the workflow that you requested has completed. 🎉
|
cartermckinnon
force-pushed
the
nodeadm-privatednsname
branch
from
0d404a2
to
572206b
Compare
|
/ci |
|
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
|
@cartermckinnon the workflow that you requested has completed. 🎉
|
cartermckinnon
force-pushed
the
nodeadm-privatednsname
branch
from
572206b
to
5395fb9
Compare
|
/ci Problem was that the aws-go-sdk-v2 doesn't detect the runtime region from IMDS automatically, you have to enable this behavior with a config option. |
|
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
|
@cartermckinnon the workflow that you requested has completed. 🎉
|
Issacwww
reviewed
Mar 12, 2024
| @@ -145,7 +145,14 @@ func (c *initCmd) Run(log *zap.Logger, opts *cli.GlobalOptions) error { | |||
| // perform in-place updates when allowed by the user | |||
| func enrichConfig(log *zap.Logger, cfg *api.NodeConfig) error { | |||
| log.Info("Fetching instance details..") | |||
| instanceDetails, err := api.GetIMDSInstanceDetails(context.TODO(), imds.New(imds.Options{})) | |||
| imdsClient := imds.New(imds.Options{}) | |||
| awsConfig, err := config.LoadDefaultConfig(context.TODO(), config.WithClientLogMode(aws.LogRetries), config.WithEC2IMDSRegion(func(o *config.UseEC2IMDSRegion) { | |||
Issacwww
approved these changes
Mar 12, 2024
ndbaker1
approved these changes
Mar 12, 2024
|
Beautiful! Thank you for this fix, @cartermckinnon. |
joelddiaz
pushed a commit
to joelddiaz/amazon-eks-ami
that referenced
this pull request
May 29, 2024
atmosx
pushed a commit
to gathertown/amazon-eks-ami
that referenced
this pull request
Jun 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#1711
Description of changes:
Sets the
--hostname-overrideon k8s 1.26+ to the EC2PrivateDnsName.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.