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

netdog: check platform-specific mechanism for getting hostname first #3021

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Apr 14, 2023

Issue number:

Addresses #3011 and potentially #3013

Description of changes:

    netdog: check platform-specific mechanism for getting hostname first
    
    Instead of looking up the hostname via reverse DNS look up first, we
    first try any platform-specific mechanism for retrieving hostnames. For
    AWS varaints, this means to query IMDS for the hostname first. This
    gives up a more authoritative hostname to set for the host.



Testing done:
Launched aws-k8s-1.26 into a subnet with hostname type set to "Resouce name".

The node comes up successfully with the node named with its resource name:

$ kubectl get nodes -o wide
NAME                                             STATUS   ROLES    AGE    VERSION               INTERNAL-IP      EXTERNAL-IP      OS-IMAGE                                KERNEL-VERSION   CONTAINER-RUNTIME
i-1234abcdef.us-west-2.compute.internal   Ready    <none>   104m   v1.26.2-eks-b106822   192.168.16.9     <none>           Bottlerocket OS 1.14.0 (aws-k8s-1.26)   5.15.102         containerd://1.6.19+bottlerocket

In older variants (aws-k8s-1.24) that still uses the in-tree AWS cloud provider code in kubelet, the node joins fine.

NAME                                             STATUS   ROLES    AGE     VERSION                INTERNAL-IP       EXTERNAL-IP      OS-IMAGE                                KERNEL-VERSION   CONTAINER-RUNTIME
i-1234abcdef.us-west-2.compute.internal   Ready    <none>   70s     v1.24.10-eks-08ad9cc   192.168.27.156    34.215.149.50    Bottlerocket OS 1.14.0 (aws-k8s-1.24)   5.15.102         containerd://1.6.19+bottlerocket

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

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

Tested with an aws-k8s-1.24 node to check for any regression issues.

NAME                                           STATUS   ROLES    AGE    VERSION                INTERNAL-IP      EXTERNAL-IP      OS-IMAGE                                KERNEL-VERSION   CONTAINER-RUNTIME
ip-192-168-94-139.us-east-2.compute.internal   Ready    <none>   4m2s   v1.24.10-eks-08ad9cc   192.168.94.139   18.216.220.116   Bottlerocket OS 1.14.0 (aws-k8s-1.24)   5.15.102         containerd://1.6.19+bottlerocket

$ k apply -f ../testing/test-pod.yaml
pod/webserver created

$ k get pod
NAME READY STATUS RESTARTS AGE
webserver 1/1 Running 0 7s


No issues found. Joined cluster fine and appears to be operational.

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

This seems ok. For non-AWS variants the platform-specific call is essentially a no-op and then the logic remains the same. I'm not sure there exists a case in AWS where you wouldn't want to use the IMDS supplied hostname? 🤔

There are a few spelling errors in the commit message:

For aws varaints should be For AWS variants

and

more authoratative should be more authoritative

Instead of looking up the hostname via reverse DNS look up first, we
first try any platform-specific mechanism for retrieving hostnames. For
AWS varaints, this means to query IMDS for the hostname first. This
gives up a more authoritative hostname to set for the host.
@etungsten
Copy link
Contributor Author

Push above fixes commit message.

@etungsten etungsten merged commit 2252095 into bottlerocket-os:develop Apr 14, 2023
37 checks passed
@etungsten etungsten deleted the hostnaem branch April 14, 2023 21:09
etungsten added a commit that referenced this pull request Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants